Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: add CHANGELOG check and specific targets #456

Merged
merged 10 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/changelog.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Runs changelog related jobs.
# CI job heavily inspired by: https://github.com/tarides/changelog-check-action

name: changelog

on:
pull_request:
types: [opened, reopened, synchronize, labeled, unlabeled]

jobs:
changelog:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@main
with:
fetch-depth: 0
- name: Check for changes in changelog
env:
BASE_REF: ${{ github.event.pull_request.base.ref }}
NO_CHANGELOG_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'no changelog') }}
run: ./scripts/check-changelog.sh "${{ inputs.changelog }}"
shell: bash
12 changes: 11 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ FEATURES_CLIENT="testing, concurrent"
FEATURES_CLI="testing, concurrent"
NODE_FEATURES_TESTING="testing"
WARNINGS=RUSTDOCFLAGS="-D warnings"
NODE_BRANCH="main"
NODE_BRANCH="next"

# --- Linting -------------------------------------------------------------------------------------

Expand Down Expand Up @@ -107,3 +107,13 @@ build: ## Builds the CLI binary and client library in release mode

build-wasm: ## Builds the client library for wasm32
cargo build --target wasm32-unknown-unknown --features idxdb,web-tonic --no-default-features --package miden-client

# --- Check ---------------------------------------------------------------------------------------

.PHONY: check
check: ## Builds the CLI binary and client library in release mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add the phony targets for these

cargo check --release --features $(FEATURES_CLI)

.PHONY: check-wasm
check-wasm: ## Builds the client library for wasm32
cargo check --target wasm32-unknown-unknown --features idxdb,web-tonic --no-default-features --package miden-client
Comment on lines +113 to +119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this has already been merged, but I would do a small follow-up PR to update the descriptions of these commands (otherwise, they are identical to the build commands).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my bad. I created this PR to fix this. Will create an issue to link it too. Sorry.

10 changes: 5 additions & 5 deletions bin/miden-cli/src/commands/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub fn show_account<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthen
"Account Hash",
"Type",
"Storage mode",
"Code Root",
"Code Commitment",
"Vault Root",
"Storage Root",
"Nonce",
Expand All @@ -135,7 +135,7 @@ pub fn show_account<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthen
account.hash().to_string(),
account_type_display_name(&account_id)?,
storage_type_display_name(&account_id),
account.code().root().to_string(),
account.code().commitment().to_string(),
account.vault().asset_tree().root().to_string(),
account.storage().root().to_string(),
account.nonce().as_int().to_string(),
Expand Down Expand Up @@ -213,14 +213,14 @@ pub fn show_account<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthen
// Code related table
{
let module = account.code().module();
let procedure_digests = account.code().procedures();
let procedures = account.code().procedures();

println!("Account Code Info:");

let mut table = create_dynamic_table(&["Procedure Digests"]);

for digest in procedure_digests {
table.add_row(vec![digest.to_hex()]);
for proc_info in procedures {
table.add_row(vec![proc_info.mast_root()]);
}
println!("{table}\n");

Expand Down
2 changes: 1 addition & 1 deletion crates/rust-client/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ pub async fn insert_mock_data(client: &mut MockClient) -> Vec<BlockHeader> {
init_seed,
account.account_type(),
miden_objects::accounts::AccountStorageType::OffChain,
account.code().root(),
account.code().commitment(),
account.storage().root(),
)
.unwrap();
Expand Down
9 changes: 4 additions & 5 deletions crates/rust-client/src/store/sqlite_store/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type SerializedAccountAuthParts = (i64, Vec<u8>);

type SerializedAccountVaultData = (String, String);

type SerializedAccountCodeData = (String, String, Vec<u8>);
type SerializedAccountCodeData = (String, Vec<u8>, Vec<u8>);

type SerializedAccountStorageData = (String, Vec<u8>);

Expand Down Expand Up @@ -315,7 +315,7 @@ pub(super) fn parse_account(
/// Serialized the provided account into database compatible types.
fn serialize_account(account: &Account) -> Result<SerializedAccountData, StoreError> {
let id: u64 = account.id().into();
let code_root = account.code().root().to_string();
let code_root = account.code().commitment().to_string();
let storage_root = account.storage().root().to_string();
let vault_root = serde_json::to_string(&account.vault().commitment())
.map_err(StoreError::InputSerializationError)?;
Expand Down Expand Up @@ -364,9 +364,8 @@ fn serialize_account_auth(
fn serialize_account_code(
account_code: &AccountCode,
) -> Result<SerializedAccountCodeData, StoreError> {
let root = account_code.root().to_string();
let procedures = serde_json::to_string(account_code.procedures())
.map_err(StoreError::InputSerializationError)?;
let root = account_code.commitment().to_string();
let procedures = account_code.procedures().to_bytes();
let module = account_code.module().to_bytes(AstSerdeOptions { serialize_imports: true });

Ok((root, procedures, module))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ extern "C" {
#[wasm_bindgen(js_name = insertAccountCode)]
pub fn idxdb_insert_account_code(
code_root: String,
code: String,
code: Vec<u8>,
module: Vec<u8>,
) -> js_sys::Promise;

Expand Down
3 changes: 2 additions & 1 deletion crates/rust-client/src/store/web_store/accounts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ impl WebStore {
) -> Result<(Account, Option<Word>), StoreError> {
let (account_stub, seed) = self.get_account_stub(account_id).await.unwrap();
let (_procedures, module_ast) =
self.get_account_code(account_stub.code_root()).await.unwrap();
self.get_account_code(account_stub.code_commitment()).await.unwrap();

let account_code = AccountCode::new(module_ast, &TransactionKernel::assembler()).unwrap();
let account_storage = self.get_account_storage(account_stub.storage_root()).await.unwrap();
let account_vault = self.get_vault_assets(account_stub.vault_root()).await.unwrap();
Expand Down
6 changes: 3 additions & 3 deletions crates/rust-client/src/store/web_store/accounts/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use super::{js_bindings::*, models::*};
use crate::store::StoreError;

pub async fn insert_account_code(account_code: &AccountCode) -> Result<(), ()> {
let root = account_code.root().to_string();
let procedures = serde_json::to_string(account_code.procedures()).unwrap();
let root = account_code.commitment().to_string();
let procedures = account_code.procedures().to_bytes();
let module = account_code.module().to_bytes(AstSerdeOptions { serialize_imports: true });

let promise = idxdb_insert_account_code(root, procedures, module);
Expand Down Expand Up @@ -68,7 +68,7 @@ pub async fn insert_account_record(
account_seed: Option<Word>,
) -> Result<(), ()> {
let account_id_str = account.id().to_string();
let code_root = account.code().root().to_string();
let code_root = account.code().commitment().to_string();
let storage_root = account.storage().root().to_string();
let vault_root = serde_json::to_string(&account.vault().commitment()).unwrap();
let committed = account.is_on_chain();
Expand Down
11 changes: 9 additions & 2 deletions crates/rust-client/src/store/web_store/js/accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,15 @@ export async function getAccountCode(
const moduleArrayBuffer = await codeRecord.module.arrayBuffer();
const moduleArray = new Uint8Array(moduleArrayBuffer);
const moduleBase64 = uint8ArrayToBase64(moduleArray);

// Convert the procedures Blob to an ArrayBuffer
const proceduresArrayBuffer = await codeRecord.procedures.arrayBuffer();
const proceduresArray = new Uint8Array(proceduresArrayBuffer);
const proceduresBase64 = uint8ArrayToBase64(proceduresArray);

return {
root: codeRecord.root,
procedures: codeRecord.procedures,
procedures: proceduresBase64,
module: moduleBase64,
};
} catch (error) {
Expand Down Expand Up @@ -349,11 +355,12 @@ export async function insertAccountCode(
try {
// Create a Blob from the ArrayBuffer
const moduleBlob = new Blob([new Uint8Array(module)]);
const codeBlob = new Blob([new Uint8Array(code)]);

// Prepare the data object to insert
const data = {
root: codeRoot, // Using codeRoot as the key
procedures: code,
procedures: codeBlob,
module: moduleBlob, // Blob created from ArrayBuffer
};

Expand Down
4 changes: 2 additions & 2 deletions crates/rust-client/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ async fn insert_basic_account() {
assert_eq!(account.nonce(), fetched_account.nonce());
assert_eq!(account.vault(), fetched_account.vault());
assert_eq!(account.storage().root(), fetched_account.storage().root());
assert_eq!(account.code().root(), fetched_account.code().root());
assert_eq!(account.code().commitment(), fetched_account.code().commitment());

// Validate seed matches
assert_eq!(account_seed, fetched_account_seed.unwrap());
Expand Down Expand Up @@ -139,7 +139,7 @@ async fn insert_faucet_account() {
assert_eq!(account.nonce(), fetched_account.nonce());
assert_eq!(account.vault(), fetched_account.vault());
assert_eq!(account.storage(), fetched_account.storage());
assert_eq!(account.code().root(), fetched_account.code().root());
assert_eq!(account.code().commitment(), fetched_account.code().commitment());

// Validate seed matches
assert_eq!(account_seed, fetched_account_seed.unwrap());
Expand Down
6 changes: 3 additions & 3 deletions crates/rust-client/src/transactions/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,9 @@ impl SwapTransactionData {
// ================================================================================================

pub mod known_script_roots {
pub const P2ID: &str = "0x3df15bd183c3239332dcb535c6d0a25c668ead19a317fefe66fc2754e49ce4f1";
pub const P2IDR: &str = "0xf6513a4c607de61288263e1d9346889e9393f3c4024bfb42efc0e2ce3c64ee72";
pub const SWAP: &str = "0x5040bdb39e3e71d8ae4a93d65ff44d152f56192df97018a63b6b6342e87f97d5";
pub const P2ID: &str = "0x07db8e6726c0859648a4f0ad38376440c01d98674b7d5a03d7ad729ae2a21d8f";
pub const P2IDR: &str = "0xd43b69d65bbc22abf64dbae53ad22e3a4f6d5bfac8e47497b69c116824b46427";
pub const SWAP: &str = "0x9270b8c89303cf7a05340351d7f9962a9722c4f35d30b7d4980929b381e5d695";
}

// TESTS
Expand Down
1 change: 0 additions & 1 deletion rust-toolchain

This file was deleted.

5 changes: 5 additions & 0 deletions rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[toolchain]
channel = "1.78"
components = ["rustfmt", "rust-src", "clippy"]
targets = ["wasm32-unknown-unknown"]
profile = "minimal"
21 changes: 21 additions & 0 deletions scripts/check-changelog.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/bin/bash
set -uo pipefail

CHANGELOG_FILE="${1:-CHANGELOG.md}"

if [ "${NO_CHANGELOG_LABEL}" = "true" ]; then
# 'no changelog' set, so finish successfully
echo "\"no changelog\" label has been set"
exit 0
else
# a changelog check is required
# fail if the diff is empty
if git diff --exit-code "origin/${BASE_REF}" -- "${CHANGELOG_FILE}"; then
>&2 echo "Changes should come with an entry in the \"CHANGELOG.md\" file. This behavior
can be overridden by using the \"no changelog\" label, which is used for changes
that are trivial / explicitely stated not to require a changelog entry."
exit 1
fi

echo "The \"CHANGELOG.md\" file has been updated."
fi
3 changes: 0 additions & 3 deletions tests/config/miden-node.toml
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
[block_producer]
# port defined as: sum(ord(c)**p for (p, c) in enumerate('miden-block-producer', 1)) % 2**16
endpoint = { host = "localhost", port = 48046 }
store_url = "http://localhost:28943"
# enables or disables the verification of transaction proofs before they are accepted into the
# transaction queue.
verify_tx_proofs = true

[rpc]
# port defined as: sum(ord(c)**p for (p, c) in enumerate('miden-rpc', 1)) % 2**16
endpoint = { host = "0.0.0.0", port = 57291 }
block_producer_url = "http://localhost:48046"
store_url = "http://localhost:28943"

[store]
# port defined as: sum(ord(c)**p for (p, c) in enumerate('miden-store', 1)) % 2**16
Expand Down