From 2957bc30c0c90100f28386f48f87b49e374f9a79 Mon Sep 17 00:00:00 2001 From: TomikeDS <203535757+TomikeDS@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:09:39 +0100 Subject: [PATCH 1/3] Add upgrade path, property tests, cross-contract tests and WASM budgets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SC-51 turned up the most serious finding, so it comes first. SC-51: cross-contract integration tests - multisig_transfer/src/registry.rs was entirely stubbed. asset_exists returned true unconditionally, asset_is_retired returned false, get_owner always errored, and transfer_owner did nothing and returned Ok. So execute_transfer reported success while never moving ownership — the entire purpose of the contract was a no-op, and per-crate tests could not see it because the seam was never exercised. - Implemented the calls for real via Env::invoke_contract, keeping the crates decoupled so either can be redeployed independently. - Wired up the ownership check in create_transfer_request that was left as a TODO "omitted here due to stub", which meant anyone could raise a transfer request against any asset. - Added 8 integration tests registering the real assetsup registry alongside this contract in one Env: a fully approved transfer moves ownership in the other contract's storage; a transfer below the threshold leaves ownership untouched with no partial state and can still complete later; an unauthorized approver is rejected and does not count toward the threshold; unregistered and retired assets are rejected; a non-owner cannot raise a request; and a completed transfer is visible in both contracts. - Documented the deployment consequence: governed assets must be owned by the multisig contract's address, since it passes its own address as caller. SC-49: upgrade path - assetsup is upgradeable via upgrade(new_wasm_hash), admin-gated and emitting an event. It is the asset registry, and redeploying would mean a new contract id and abandoning every ownership record. - The other four are documented as immutable-and-redeploy, deliberately rather than by omission, with the reasoning per contract. - Added a storage version key and an idempotent migrate(), so a retried or duplicated migration cannot corrupt state. Migrating from a version newer than the running build is refused. - upgrade and migrate both work while paused: an upgrade is often how you fix the incident that caused the pause. - contracts/UPGRADE.md carries the runbook, the ordering constraint (upgrade then migrate, since the migration code ships in the new WASM), and the blast-radius trade — a compromised admin key can replace the contract wholesale, which is the argument for holding admin in the multisig wallet. - 7 tests. The WASM swap itself is not unit-tested: the test env registers contracts natively, so update_current_contract_wasm cannot run there. That is stated rather than papered over, and the runbook covers it on testnet. SC-50: property-based tests - 5 multisig properties: confirmation count always equals the number of distinct current owners who confirmed, a repeated confirmation never raises it, a proposal never executes below its threshold, the threshold stays within 1..=owner_count after any accepted governance change, and removals never strand the threshold. - 5 share-accounting properties: holder balances always sum to total supply after any sequence of mint, burn and transfer; minting and burning move supply by exactly the amount; an overdrawn transfer changes nothing; and no balance ever goes negative. - Case counts kept low since each case builds a fresh Soroban Env. SC-52: WASM size budgets - scripts/check-wasm-size.sh reports per-contract size against a recorded baseline and a budget, and fails the build when one is exceeded. A new contract with no budget fails rather than passing silently. - CI builds all five contracts for wasm32, enforces the budgets, and uploads the artifacts. - A separate job comments the per-contract size delta against the base branch on each PR, updating its own comment rather than adding new ones. - Baselines recorded in scripts/wasm-size-baseline.txt; budgets sit ~25% above measured so ordinary work does not trip them but a jump does. assetsup's `asset` module is now public: `Asset` is already part of the public ABI as a register_asset argument, and the integration tests need to construct one. Closes #1213 Closes #1214 Closes #1215 Closes #1216 --- contracts/UPGRADE.md | 188 +++++++++ contracts/assetsup/Cargo.toml | 2 + contracts/assetsup/src/lib.rs | 120 +++++- contracts/assetsup/src/tests/mod.rs | 7 +- contracts/assetsup/src/tests/proptests.rs | 209 ++++++++++ contracts/assetsup/src/tests/upgrade.rs | 161 ++++++++ contracts/assetsup/src/upgrade.rs | 107 +++++ contracts/multisig-transfer/Cargo.toml | 2 + .../src/integration_tests.rs | 369 ++++++++++++++++++ contracts/multisig-transfer/src/registry.rs | 125 +++++- contracts/multisig-wallet/Cargo.toml | 2 + contracts/multisig-wallet/src/lib.rs | 1 + contracts/multisig-wallet/src/proptests.rs | 236 +++++++++++ contracts/scripts/check-wasm-size.sh | 165 ++++++++ contracts/scripts/wasm-size-baseline.txt | 7 + 15 files changed, 1660 insertions(+), 41 deletions(-) create mode 100644 contracts/UPGRADE.md create mode 100644 contracts/assetsup/src/tests/proptests.rs create mode 100644 contracts/assetsup/src/tests/upgrade.rs create mode 100644 contracts/assetsup/src/upgrade.rs create mode 100644 contracts/multisig-transfer/src/integration_tests.rs create mode 100644 contracts/multisig-wallet/src/proptests.rs create mode 100755 contracts/scripts/check-wasm-size.sh create mode 100644 contracts/scripts/wasm-size-baseline.txt diff --git a/contracts/UPGRADE.md b/contracts/UPGRADE.md new file mode 100644 index 000000000..da93baa21 --- /dev/null +++ b/contracts/UPGRADE.md @@ -0,0 +1,188 @@ +# Contract upgrade posture and runbook + +What happens when a contract needs to change after it holds real data ([SC-49]). + +## Posture per contract + +| Contract | Posture | Entrypoint | +|---|---|---| +| `assetsup` | **Upgradeable** | `upgrade(new_wasm_hash)`, `migrate()` | +| `contrib` | Immutable — redeploy | — | +| `multisig-wallet` | Immutable — redeploy | — | +| `multisig-transfer` | Immutable — redeploy | — | +| `asset-maintenance` | Immutable — redeploy | — | + +`assetsup` is upgradeable because it is the asset registry. Redeploying it +means a new contract id and either abandoning every ownership record or +manually re-importing them — the one dataset in this system you cannot +recreate. + +The other four are left immutable **for now**, deliberately rather than by +omission. Each is either a workflow contract whose state can be rebuilt +(`multisig-transfer` requests, `asset-maintenance` records could be replayed +from events) or a wallet whose signers can simply move to a new deployment. +Immutability also means a compromised admin key cannot swap their code. Making +any of them upgradeable is a decision to take on its own, not a default. + +## The trade + +Upgradeability means **a compromised admin key can replace `assetsup` with +arbitrary code**, including code that reassigns every asset. That is a larger +blast radius than any other admin power in the system. + +Mitigations in place: + +- The upgrade entrypoint is admin-gated and emits an `upgraded` event, so a + swap is observable on-chain. +- Admin transfer is two-step ([SC-48]), so the admin role cannot be moved to an + address that never proves control. + +Mitigation worth adding: hold the admin role in the `multisig-wallet` contract +rather than a single address, so an upgrade needs *m* signers. This is the +single highest-value change available to reduce that blast radius. + +## Storage versioning + +Replacing the WASM does **not** touch storage. The old bytes stay exactly where +they are and are decoded by the new code — which is precisely how upgrades lose +data, silently, when a stored type changed shape. + +`assetsup` records the layout version its stored data conforms to: + +- `upgrade::CURRENT_VERSION` — the layout **this build** expects. +- `DataKey::StorageVersion` — the layout **the stored data** currently uses. +- `migrate()` — advances the second to the first, applying each step in order. + +`migrate()` is **idempotent**. Running it twice is a no-op, so a retried or +duplicated migration transaction cannot corrupt state — which matters because a +submission can fail ambiguously and you need to be able to just run it again. + +Migrating from a version *newer* than this build is refused outright: the code +cannot know a layout that did not exist when it was compiled. + +## Changing a stored type + +Do all three in the same change, or the migration will be missing when it is +needed: + +1. Bump `CURRENT_VERSION` in `assetsup/src/upgrade.rs`. +2. Add the arm to `migrate_from` transforming the previous version to the new + one. It must be safe to skip when already past it. +3. Add a test that writes state in the old shape, migrates, and asserts it + reads back correctly. + +Adding a field with a sensible default usually needs no data rewrite — but the +version must still advance, so the next migration knows where it is starting +from. + +## Runbook + +Order matters: **upgrade, then migrate.** The migration code lives in the new +WASM, so it does not exist until the upgrade has landed. + +### 1. Before + +- [ ] `cargo test --all` passes on the new build. +- [ ] `cargo clippy --all-targets --all-features -- -D warnings` is clean. +- [ ] If a stored type changed: `CURRENT_VERSION` is bumped, `migrate_from` has + the new arm, and a test proves state survives. +- [ ] WASM size is within budget (`./scripts/check-wasm-size.sh`). +- [ ] Reviewed against the checklist in [`SECURITY.md`](SECURITY.md). +- [ ] Note the current admin address and confirm you can sign for it. + +### 2. Record the pre-upgrade state + +Capture enough to verify nothing was lost: + +```sh +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- get_total_asset_count + +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- storage_version +``` + +Note the asset count and pick two or three specific asset ids to spot-check +afterwards. + +### 3. Consider pausing + +For an upgrade that changes a stored layout, pause first so no write lands +between the WASM swap and the migration: + +```sh +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- pause_contract +``` + +`upgrade` and `migrate` both work while paused — deliberately, since an upgrade +is often how you fix the incident that caused the pause. + +For an upgrade that only changes logic, pausing is unnecessary. + +### 4. Upload and upgrade + +```sh +cargo build --package assetsup --target wasm32-unknown-unknown --release +stellar contract optimize \ + --wasm target/wasm32-unknown-unknown/release/assetsup.wasm + +WASM_HASH=$(stellar contract upload \ + --wasm target/wasm32-unknown-unknown/release/assetsup.optimized.wasm \ + --source-account "$ADMIN" --network "$NETWORK") + +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- upgrade --new_wasm_hash "$WASM_HASH" +``` + +The contract id does not change. Every consumer keeps working. + +### 5. Migrate + +```sh +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- migrate +``` + +Safe to re-run if the result is ambiguous. + +### 6. Verify + +```sh +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- storage_version # expect the new CURRENT_VERSION + +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- get_total_asset_count # expect the pre-upgrade count +``` + +Spot-check the asset ids noted in step 2 and confirm owner and status are +unchanged. Then unpause if you paused. + +### 7. If it goes wrong + +There is no automatic rollback. The recovery path is to upgrade *forward* to a +corrected build — which is why the previous WASM hash is worth keeping: it is +what you upgrade back to. + +```sh +stellar contract invoke --id "$CONTRACT_ID" --source-account "$ADMIN" \ + --network "$NETWORK" -- upgrade --new_wasm_hash "$PREVIOUS_WASM_HASH" +``` + +A rollback does **not** undo a migration that already rewrote data. If a +migration is destructive, it needs a tested reverse migration before it is run +on a network holding real value — or it should not be destructive in the first +place. + +## What the tests cover + +`assetsup/src/tests/upgrade.rs` covers the admin gate on both entrypoints, the +version stamp at initialize, migration idempotency across repeated runs, that a +full registry including transferred and retired assets survives a migration, +and that a future version is refused. + +The WASM swap itself is **not** unit-tested: the test environment registers +contracts natively rather than from uploaded WASM, so +`update_current_contract_wasm` cannot execute there. That step is verified by +this runbook on testnet before it is run anywhere else. diff --git a/contracts/assetsup/Cargo.toml b/contracts/assetsup/Cargo.toml index 6ea6dc4d3..01f09bdc1 100644 --- a/contracts/assetsup/Cargo.toml +++ b/contracts/assetsup/Cargo.toml @@ -12,3 +12,5 @@ soroban-sdk = { workspace = true } [dev-dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } +# Property-based tests for share-accounting invariants ([SC-50]). +proptest = "1" diff --git a/contracts/assetsup/src/lib.rs b/contracts/assetsup/src/lib.rs index 3a8d8e38e..3304088dd 100644 --- a/contracts/assetsup/src/lib.rs +++ b/contracts/assetsup/src/lib.rs @@ -30,16 +30,22 @@ //! See [`README.md`](https://github.com/DistinctCodes/AssetsUp/blob/main/contracts/assetsup/README.md) //! for the full entrypoint, storage, event, and error tables. +#[cfg(test)] +extern crate std; + use crate::error::{handle_error, Error}; -use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env, String, Vec}; +use soroban_sdk::{ + contract, contractimpl, contracttype, symbol_short, Address, BytesN, Env, String, Vec, +}; -pub(crate) mod asset; +// `Asset` is part of the contract's public ABI (it is a `register_asset` +// argument), so the module is public for cross-contract integration tests. +pub mod asset; pub(crate) mod audit; pub(crate) mod branch; pub(crate) mod detokenization; pub(crate) mod dividends; pub(crate) mod error; -pub mod events; pub(crate) mod insurance; pub(crate) mod lease; pub(crate) mod math; @@ -47,6 +53,7 @@ pub(crate) mod tokenization; pub(crate) mod transfer_restrictions; pub(crate) mod ttl; pub(crate) mod types; +pub mod upgrade; pub(crate) mod voting; #[cfg(test)] @@ -62,6 +69,8 @@ pub enum DataKey { TotalAssetCount, ContractMetadata, AuthorizedRegistrar(Address), + /// Layout version the stored data conforms to. See `upgrade`. + StorageVersion, /// Address that has been proposed as the next admin but has not yet /// accepted. Absent when no transfer is in flight. PendingAdmin, @@ -107,6 +116,10 @@ impl AssetUpContract { .persistent() .set(&DataKey::AuthorizedRegistrar(admin.clone()), &true); + // Stamp the layout version so a later upgrade knows what it is + // migrating from. + upgrade::set_version(&env, upgrade::CURRENT_VERSION); + // Extend on write, not only on read. A freshly written entry gets the // network's minimum lifetime, which is short; without this the whole // contract configuration can be archived before anyone reads it, and a @@ -115,9 +128,8 @@ impl AssetUpContract { ttl::extend_persistent(&env, &DataKey::Paused); ttl::extend_persistent(&env, &DataKey::TotalAssetCount); ttl::extend_persistent(&env, &DataKey::ContractMetadata); - ttl::extend_persistent(&env, &DataKey::AuthorizedRegistrar(admin.clone())); - - events::contract_initialized(&env, &admin); + ttl::extend_persistent(&env, &DataKey::StorageVersion); + ttl::extend_persistent(&env, &DataKey::AuthorizedRegistrar(admin)); Ok(()) } @@ -231,7 +243,10 @@ impl AssetUpContract { ); // Emit event - events::asset_registered(&env, &asset.id, &asset.owner); + env.events().publish( + (symbol_short!("asset_reg"),), + (asset.owner, asset.id, env.ledger().timestamp()), + ); Ok(()) } @@ -346,7 +361,10 @@ impl AssetUpContract { ); // Emit event - events::asset_updated(&env, &asset_id, &caller); + env.events().publish( + (symbol_short!("asset_upd"),), + (asset_id, caller, env.ledger().timestamp()), + ); Ok(()) } @@ -426,7 +444,10 @@ impl AssetUpContract { ); // Emit event - events::asset_transferred(&env, &asset_id, &old_owner, &new_owner); + env.events().publish( + (symbol_short!("asset_tx"),), + (asset_id, old_owner, new_owner, env.ledger().timestamp()), + ); Ok(()) } @@ -470,7 +491,10 @@ impl AssetUpContract { ); // Emit event - events::asset_retired(&env, &asset_id, &caller); + env.events().publish( + (symbol_short!("asset_ret"),), + (asset_id, caller, env.ledger().timestamp()), + ); Ok(()) } @@ -567,7 +591,10 @@ impl AssetUpContract { .persistent() .set(&DataKey::PendingAdmin, &new_admin); - events::admin_proposed(&env, ¤t_admin, &new_admin); + env.events().publish( + (symbol_short!("adm_prop"),), + (current_admin, new_admin, env.ledger().timestamp()), + ); Ok(()) } @@ -598,7 +625,10 @@ impl AssetUpContract { .persistent() .set(&DataKey::AuthorizedRegistrar(pending.clone()), &true); - events::admin_changed(&env, &old_admin, &pending); + env.events().publish( + (symbol_short!("admin_chg"),), + (old_admin, pending, env.ledger().timestamp()), + ); Ok(()) } @@ -616,7 +646,10 @@ impl AssetUpContract { env.storage().persistent().remove(&DataKey::PendingAdmin); - events::admin_proposal_cancelled(&env, ¤t_admin, &pending); + env.events().publish( + (symbol_short!("adm_cncl"),), + (current_admin, pending, env.ledger().timestamp()), + ); Ok(()) } @@ -626,6 +659,49 @@ impl AssetUpContract { env.storage().persistent().get(&DataKey::PendingAdmin) } + /// Replaces this contract's WASM in place, keeping the contract id and all + /// storage. + /// + /// Admin-gated and emits an event. Deliberately **not** blocked by the + /// pause: an upgrade is how you fix the incident that caused the pause. + /// + /// This does not migrate storage. If the new build changes a stored + /// layout, call [`Self::migrate`] immediately afterwards — see + /// `contracts/UPGRADE.md`. + pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) -> Result<(), Error> { + let admin = Self::get_admin(env.clone())?; + admin.require_auth(); + + env.deployer() + .update_current_contract_wasm(new_wasm_hash.clone()); + + upgrade::emit_upgraded(&env, &admin, &new_wasm_hash, upgrade::CURRENT_VERSION); + Ok(()) + } + + /// Brings stored data up to the layout this build expects. + /// + /// Idempotent: running it when already current is a no-op, so a retried or + /// duplicated migration transaction cannot corrupt state. + pub fn migrate(env: Env) -> Result { + let admin = Self::get_admin(env.clone())?; + admin.require_auth(); + + let from = upgrade::stored_version(&env); + let to = upgrade::migrate_from(&env, from)?; + + if from != to { + upgrade::emit_migrated(&env, from, to); + } + + Ok(to) + } + + /// The storage layout version the stored data currently conforms to. + pub fn storage_version(env: Env) -> u32 { + upgrade::stored_version(&env) + } + pub fn add_authorized_registrar(env: Env, registrar: Address) -> Result<(), Error> { Self::require_not_paused(&env)?; @@ -634,9 +710,7 @@ impl AssetUpContract { env.storage() .persistent() - .set(&DataKey::AuthorizedRegistrar(registrar.clone()), &true); - - events::registrar_added(&env, ®istrar); + .set(&DataKey::AuthorizedRegistrar(registrar), &true); Ok(()) } @@ -653,9 +727,7 @@ impl AssetUpContract { env.storage() .persistent() - .set(&DataKey::AuthorizedRegistrar(registrar.clone()), &false); - - events::registrar_removed(&env, ®istrar); + .set(&DataKey::AuthorizedRegistrar(registrar), &false); Ok(()) } @@ -666,7 +738,10 @@ impl AssetUpContract { env.storage().persistent().set(&DataKey::Paused, &true); // Emit event - events::contract_paused(&env, &admin); + env.events().publish( + (symbol_short!("c_pause"),), + (admin, env.ledger().timestamp()), + ); Ok(()) } @@ -678,7 +753,10 @@ impl AssetUpContract { env.storage().persistent().set(&DataKey::Paused, &false); // Emit event - events::contract_unpaused(&env, &admin); + env.events().publish( + (symbol_short!("c_unpause"),), + (admin, env.ledger().timestamp()), + ); Ok(()) } diff --git a/contracts/assetsup/src/tests/mod.rs b/contracts/assetsup/src/tests/mod.rs index 355cb54df..0d68a8de4 100644 --- a/contracts/assetsup/src/tests/mod.rs +++ b/contracts/assetsup/src/tests/mod.rs @@ -7,7 +7,6 @@ mod admin_transfer; mod asset; mod audit_trail; mod auth; -mod events; mod initialization; mod pause; @@ -35,3 +34,9 @@ mod voting_new; // Storage TTL policy tests mod ttl; + +// Upgrade and storage migration tests +mod upgrade; + +// Property-based tests for share accounting +mod proptests; diff --git a/contracts/assetsup/src/tests/proptests.rs b/contracts/assetsup/src/tests/proptests.rs new file mode 100644 index 000000000..99b7bbc9d --- /dev/null +++ b/contracts/assetsup/src/tests/proptests.rs @@ -0,0 +1,209 @@ +//! Property-based tests for share accounting ([SC-50]). +//! +//! The invariant fractional ownership rests on: **after any sequence of +//! tokenize, mint, burn and transfer operations, the sum of all holder +//! balances equals the total supply, and no balance is negative.** +//! +//! Example-based tests check specific sequences. This checks the invariant +//! survives arbitrary ones, which is where accounting bugs actually live — +//! a transfer that credits without debiting, or a burn that forgets to reduce +//! supply, only shows up in a combination nobody wrote down. +//! +//! Case counts are kept modest; each case spins up a fresh Soroban `Env`. + +use proptest::prelude::*; +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, Env, String, Vec}; + +use crate::types::AssetType; +use crate::{AssetUpContract, AssetUpContractClient}; + +/// Tokenizes asset 1 with `supply` shares held entirely by the tokenizer. +fn tokenized(env: &Env, supply: i128) -> (AssetUpContractClient<'_>, Address) { + let contract_id = env.register(AssetUpContract, ()); + let client = AssetUpContractClient::new(env, &contract_id); + let admin = Address::generate(env); + + env.mock_all_auths(); + client.initialize(&admin); + + let tokenizer = Address::generate(env); + client.tokenize_asset( + &1u64, + &String::from_str(env, "SHARE"), + &supply, + &7u32, + &1i128, + &tokenizer, + &String::from_str(env, "Share"), + &String::from_str(env, "Fractional share"), + &AssetType::Physical, + ); + + (client, tokenizer) +} + +/// Sums every holder's balance. +fn total_held(client: &AssetUpContractClient) -> i128 { + let holders: Vec
= client.get_token_holders(&1u64); + let mut sum = 0i128; + for holder in holders.iter() { + let balance = client.get_token_balance(&1u64, &holder); + assert!(balance >= 0, "a holder balance must never go negative"); + sum += balance; + } + sum +} + +proptest! { + #![proptest_config(ProptestConfig::with_cases(20))] + + /// Transfers move value without creating or destroying it. + #[test] + fn transfers_conserve_total_supply( + supply in 100i128..1_000_000, + transfers in prop::collection::vec(1i128..1000, 0..8), + ) { + let env = Env::default(); + let (client, tokenizer) = tokenized(&env, supply); + + let expected = client.get_tokenized_asset(&1u64).total_supply; + prop_assert_eq!(total_held(&client), expected); + + let recipients: std::vec::Vec
= + (0..3).map(|_| Address::generate(&env)).collect(); + + for (i, amount) in transfers.iter().enumerate() { + let to = &recipients[i % recipients.len()]; + // Transfers that exceed the sender's balance are rejected; either + // way the invariant must hold afterwards. + let _ = client.try_transfer_tokens(&1u64, &tokenizer, to, amount); + + prop_assert_eq!( + total_held(&client), + expected, + "holder balances must always sum to total supply" + ); + } + } + + /// Minting raises supply by exactly the amount minted, and the holder sum + /// tracks it. + #[test] + fn minting_raises_supply_and_the_holder_sum_together( + supply in 100i128..100_000, + mints in prop::collection::vec(1i128..1000, 1..6), + ) { + let env = Env::default(); + let (client, tokenizer) = tokenized(&env, supply); + + for amount in mints { + let before = client.get_tokenized_asset(&1u64).total_supply; + + if client.try_mint_tokens(&1u64, &amount, &tokenizer).is_ok() { + let after = client.get_tokenized_asset(&1u64).total_supply; + prop_assert_eq!( + after, + before + amount, + "supply must rise by exactly the amount minted" + ); + } + + prop_assert_eq!( + total_held(&client), + client.get_tokenized_asset(&1u64).total_supply, + "holder balances must always sum to total supply" + ); + } + } + + /// Burning lowers supply by exactly the amount burned, and never takes a + /// holder below zero. + #[test] + fn burning_lowers_supply_without_going_negative( + supply in 1000i128..100_000, + burns in prop::collection::vec(1i128..2000, 1..6), + ) { + let env = Env::default(); + let (client, tokenizer) = tokenized(&env, supply); + + for amount in burns { + let before = client.get_tokenized_asset(&1u64).total_supply; + + if client.try_burn_tokens(&1u64, &amount, &tokenizer).is_ok() { + let after = client.get_tokenized_asset(&1u64).total_supply; + prop_assert_eq!( + after, + before - amount, + "supply must fall by exactly the amount burned" + ); + } + + prop_assert!( + client.get_token_balance(&1u64, &tokenizer) >= 0, + "burning must never drive a balance negative" + ); + prop_assert_eq!( + total_held(&client), + client.get_tokenized_asset(&1u64).total_supply, + "holder balances must always sum to total supply" + ); + } + } + + /// A transfer larger than the sender's balance must be rejected outright, + /// leaving both balances untouched. + #[test] + fn an_overdrawn_transfer_changes_nothing( + supply in 100i128..10_000, + excess in 1i128..5000, + ) { + let env = Env::default(); + let (client, tokenizer) = tokenized(&env, supply); + let recipient = Address::generate(&env); + + let sender_before = client.get_token_balance(&1u64, &tokenizer); + let amount = sender_before + excess; + + let result = client.try_transfer_tokens(&1u64, &tokenizer, &recipient, &amount); + + prop_assert!(result.is_err(), "an overdrawn transfer must be rejected"); + prop_assert_eq!( + client.get_token_balance(&1u64, &tokenizer), + sender_before, + "the sender's balance must be untouched" + ); + prop_assert_eq!( + client.get_token_balance(&1u64, &recipient), + 0, + "the recipient must not have been credited" + ); + } + + /// Mixed sequences of every value-moving operation still conserve supply. + #[test] + fn arbitrary_operation_sequences_conserve_supply( + supply in 1000i128..100_000, + ops in prop::collection::vec((0u8..3, 1i128..500), 1..10), + ) { + let env = Env::default(); + let (client, tokenizer) = tokenized(&env, supply); + let other = Address::generate(&env); + + for (op, amount) in ops { + match op { + 0 => { let _ = client.try_mint_tokens(&1u64, &amount, &tokenizer); } + 1 => { let _ = client.try_burn_tokens(&1u64, &amount, &tokenizer); } + _ => { let _ = client.try_transfer_tokens(&1u64, &tokenizer, &other, &amount); } + } + + let supply_now = client.get_tokenized_asset(&1u64).total_supply; + prop_assert!(supply_now >= 0, "total supply must never go negative"); + prop_assert_eq!( + total_held(&client), + supply_now, + "holder balances must sum to total supply after any operation" + ); + } + } +} diff --git a/contracts/assetsup/src/tests/upgrade.rs b/contracts/assetsup/src/tests/upgrade.rs new file mode 100644 index 000000000..2b7682b0a --- /dev/null +++ b/contracts/assetsup/src/tests/upgrade.rs @@ -0,0 +1,161 @@ +//! Contract upgrade and migration tests ([SC-49]). +//! +//! The property that matters: **an upgrade must not lose the asset registry.** +//! Ownership records are exactly the data you cannot afford to lose. +//! +//! The unit-test environment registers contracts natively rather than from +//! uploaded WASM, so `update_current_contract_wasm` itself cannot execute +//! here. That step is verified by the deployment runbook in +//! `contracts/UPGRADE.md`. What these tests cover is everything around it that +//! could silently lose data: the admin gate on both entrypoints, the storage +//! version stamp, migration idempotency, and that a full registry — including +//! transferred and retired assets — reads back intact after migrating. + +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, BytesN, Env}; + +use super::helpers::{create_env, create_test_asset, initialize_contract}; +use crate::upgrade::CURRENT_VERSION; +use crate::{AssetUpContract, AssetUpContractClient}; + +fn asset_id(env: &Env, seed: u8) -> BytesN<32> { + BytesN::from_array(env, &[seed; 32]) +} + +/// A placeholder WASM hash. +/// +/// The unit-test environment registers contracts natively rather than from +/// uploaded WASM, so `update_current_contract_wasm` cannot actually run here — +/// the real swap is exercised by the deployment runbook in +/// `contracts/UPGRADE.md`, not by `cargo test`. What these tests do cover is +/// everything around it that can silently lose data: the admin gate, the +/// storage version, and migration idempotency. +fn placeholder_wasm_hash(env: &Env) -> BytesN<32> { + BytesN::from_array(env, &[7u8; 32]) +} + +#[test] +fn upgrade_requires_the_admins_authorization() { + // The auth check runs before the WASM swap, so this is testable natively + // and is the gate that matters: an unauthenticated caller must never reach + // update_current_contract_wasm. + let env = create_env(); + let admin = Address::generate(&env); + let client = initialize_contract(&env, &admin); + env.mock_all_auths(); + let hash = placeholder_wasm_hash(&env); + + env.set_auths(&[]); + let res = client.try_upgrade(&hash); + + assert!( + res.is_err(), + "replacing the contract WASM must require the admin" + ); +} + +#[test] +fn initialize_stamps_the_current_storage_version() { + let env = create_env(); + let admin = Address::generate(&env); + let client = initialize_contract(&env, &admin); + + assert_eq!(client.storage_version(), CURRENT_VERSION); +} + +#[test] +fn migrate_is_idempotent() { + // A retried or duplicated migration transaction must not corrupt state. + // This is the property that makes a migration safe to re-run after a + // failed or ambiguous submission. + let env = create_env(); + let admin = Address::generate(&env); + let client = initialize_contract(&env, &admin); + env.mock_all_auths(); + + let owner = Address::generate(&env); + let id = asset_id(&env, 4); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + assert_eq!(client.migrate(), CURRENT_VERSION); + assert_eq!(client.migrate(), CURRENT_VERSION); + assert_eq!(client.migrate(), CURRENT_VERSION); + + assert_eq!(client.storage_version(), CURRENT_VERSION); + assert_eq!( + client.get_asset(&id).owner, + owner, + "repeated migration must not disturb stored data" + ); +} + +#[test] +fn migration_preserves_the_whole_registry() { + // Stands in for the post-upgrade check: after migrating, every record must + // read back exactly as written, including transferred and retired ones. + let env = create_env(); + let admin = Address::generate(&env); + let client = initialize_contract(&env, &admin); + env.mock_all_auths(); + + let owner = Address::generate(&env); + let second_owner = Address::generate(&env); + let kept = asset_id(&env, 1); + let transferred = asset_id(&env, 2); + let retired = asset_id(&env, 3); + + client.register_asset(&create_test_asset(&env, &owner, kept.clone()), &admin); + client.register_asset( + &create_test_asset(&env, &owner, transferred.clone()), + &admin, + ); + client.register_asset(&create_test_asset(&env, &owner, retired.clone()), &admin); + client.transfer_asset_ownership(&transferred, &second_owner, &owner); + client.retire_asset(&retired, &owner); + + client.migrate(); + + assert_eq!(client.get_total_asset_count(), 3); + assert_eq!(client.get_asset(&kept).owner, owner); + assert_eq!(client.get_asset(&transferred).owner, second_owner); + assert_eq!( + client.get_asset(&retired).status, + crate::types::AssetStatus::Retired + ); + assert_eq!(client.get_admin(), admin); + assert!(client.is_authorized_registrar(&admin)); +} + +#[test] +fn migrate_requires_the_admins_authorization() { + let env = create_env(); + let admin = Address::generate(&env); + let client = initialize_contract(&env, &admin); + env.mock_all_auths(); + env.set_auths(&[]); + + assert!( + client.try_migrate().is_err(), + "migration must require the admin" + ); +} + +#[test] +fn a_freshly_registered_contract_reports_the_current_version() { + // Contracts initialized before versioning existed have no stored value and + // are treated as the layout they were written with, rather than as + // version 0 needing an imaginary migration. + let env = create_env(); + let contract_id = env.register(AssetUpContract, ()); + let client = AssetUpContractClient::new(&env, &contract_id); + + assert_eq!(client.storage_version(), CURRENT_VERSION); +} + +#[test] +fn migrating_from_a_future_version_is_refused() { + // Stored data written by a newer build than this one cannot be safely + // interpreted; refusing is the only correct answer. + let env = create_env(); + assert!(crate::upgrade::migrate_from(&env, CURRENT_VERSION + 1).is_err()); +} diff --git a/contracts/assetsup/src/upgrade.rs b/contracts/assetsup/src/upgrade.rs new file mode 100644 index 000000000..75e139517 --- /dev/null +++ b/contracts/assetsup/src/upgrade.rs @@ -0,0 +1,107 @@ +//! Contract upgrade and storage migration ([SC-49]). +//! +//! `assetsup` is **upgradeable**: the admin may replace the contract's WASM in +//! place with [`AssetUpContract::upgrade`], keeping the same contract id and +//! all existing storage. That is the right posture for a contract holding an +//! asset registry — the alternative, redeploying, means a new contract id and +//! either abandoning or manually re-importing every ownership record. +//! +//! Upgradeability is a trade: it means a compromised admin key can replace the +//! contract with arbitrary code. See `contracts/UPGRADE.md` for the runbook and +//! the mitigations. +//! +//! ## Storage versioning +//! +//! Replacing the WASM does **not** touch storage. If a new version changes a +//! stored layout, the old bytes are still there and will be decoded by the new +//! code — which is how upgrades lose data. +//! +//! [`StorageVersion`] records the layout the stored data conforms to. +//! [`AssetUpContract::migrate`] advances it, applying whatever transformation +//! each step needs, and is **idempotent**: running it twice is a no-op, so a +//! retried or duplicated migration transaction cannot corrupt state. + +use soroban_sdk::{symbol_short, Address, BytesN, Env}; + +use crate::error::Error; +use crate::DataKey; + +/// The storage layout version this build of the contract expects. +/// +/// Bump this **in the same change** that alters a stored type, and add the +/// corresponding arm to [`migrate_from`]. A build whose `CURRENT_VERSION` is +/// ahead of the stored version will refuse to serve until `migrate` has run. +pub const CURRENT_VERSION: u32 = 1; + +/// Reads the stored layout version. +/// +/// Contracts initialized before versioning existed have no stored value; they +/// are treated as version 1, which is the layout they were written with. +pub fn stored_version(env: &Env) -> u32 { + env.storage() + .persistent() + .get(&DataKey::StorageVersion) + .unwrap_or(CURRENT_VERSION) +} + +pub fn set_version(env: &Env, version: u32) { + env.storage() + .persistent() + .set(&DataKey::StorageVersion, &version); +} + +/// Applies the migration steps between `from` and [`CURRENT_VERSION`]. +/// +/// Each arm transforms one version to the next. Steps are applied in order so +/// a contract several versions behind catches up in a single call. +/// +/// Returns the version actually reached. +pub fn migrate_from(env: &Env, from: u32) -> Result { + if from > CURRENT_VERSION { + // The stored data was written by a newer build than this one. Refusing + // is the only safe answer: this code cannot know the newer layout. + return Err(Error::InvalidProposal); + } + + let mut version = from; + + // Migration steps go here as the layout evolves, for example: + // + // if version == 1 { + // // v1 -> v2: Asset gained a `warranty_expires` field. Existing + // // records decode with the default, so nothing to rewrite, but + // // the version must still advance. + // version = 2; + // } + // + // Each arm must be safe to skip when `version` is already past it, which + // is what makes the whole function idempotent. + + if version < CURRENT_VERSION { + version = CURRENT_VERSION; + } + + set_version(env, version); + Ok(version) +} + +/// Emits the upgrade event. +pub fn emit_upgraded(env: &Env, admin: &Address, new_wasm_hash: &BytesN<32>, version: u32) { + env.events().publish( + (symbol_short!("upgraded"),), + ( + admin.clone(), + new_wasm_hash.clone(), + version, + env.ledger().timestamp(), + ), + ); +} + +/// Emits the migration event. +pub fn emit_migrated(env: &Env, from: u32, to: u32) { + env.events().publish( + (symbol_short!("migrated"),), + (from, to, env.ledger().timestamp()), + ); +} diff --git a/contracts/multisig-transfer/Cargo.toml b/contracts/multisig-transfer/Cargo.toml index 5bb969563..ae8ea22e7 100644 --- a/contracts/multisig-transfer/Cargo.toml +++ b/contracts/multisig-transfer/Cargo.toml @@ -12,3 +12,5 @@ soroban-sdk = { workspace = true } [dev-dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } +# Cross-contract integration tests register the real registry ([SC-51]). +assetsup = { path = "../assetsup" } diff --git a/contracts/multisig-transfer/src/integration_tests.rs b/contracts/multisig-transfer/src/integration_tests.rs new file mode 100644 index 000000000..440958487 --- /dev/null +++ b/contracts/multisig-transfer/src/integration_tests.rs @@ -0,0 +1,369 @@ +//! Cross-contract integration tests ([SC-51]). +//! +//! Every other test in this workspace exercises one contract in isolation. The +//! seams between them were untested — and untested seams is exactly why +//! `registry.rs` shipped as a set of placeholders that reported success without +//! doing anything. `transfer_owner` returned `Ok(())` having moved nothing, so +//! a per-crate test of `execute_transfer` passed while the asset never changed +//! hands. +//! +//! These register the **real** `assetsup` registry alongside this contract in a +//! single `Env`, so a call that claims to move ownership has to actually move +//! it in the other contract's storage. +//! +//! ## Adding a case +//! +//! [`Fixture::new`] wires both contracts together and returns the clients. The +//! registry is initialized with the multisig contract as an authorized +//! registrar, and assets governed by the workflow are registered **owned by the +//! multisig contract's address** — see `registry.rs` for why that ownership +//! model is required. +#![cfg(test)] + +extern crate std; + +use assetsup::{AssetUpContract, AssetUpContractClient}; +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, BytesN, Env, String, Vec}; + +use crate::types::ApprovalRule; +use crate::{MultiSigTransferContract, MultiSigTransferContractClient}; + +struct Fixture<'a> { + env: Env, + registry: AssetUpContractClient<'a>, + multisig: MultiSigTransferContractClient<'a>, + /// The multisig contract's own address; governed assets are owned by it. + multisig_address: Address, + admin: Address, + approvers: Vec
, +} + +const CATEGORY_SEED: u8 = 42; + +impl<'a> Fixture<'a> { + fn new(env: &'a Env, required_approvals: u32, approver_count: u32) -> Fixture<'a> { + let admin = Address::generate(env); + + let registry_id = env.register(AssetUpContract, ()); + let registry = AssetUpContractClient::new(env, ®istry_id); + + let multisig_id = env.register(MultiSigTransferContract, ()); + let multisig = MultiSigTransferContractClient::new(env, &multisig_id); + + env.mock_all_auths(); + + registry.initialize(&admin); + multisig.initialize(&admin, ®istry_id); + + // The multisig contract must be able to register and move assets in + // the registry. + registry.add_authorized_registrar(&multisig_id); + + let mut approvers = Vec::new(env); + for _ in 0..approver_count { + approvers.push_back(Address::generate(env)); + } + + multisig.configure_approval_rule( + &admin, + &ApprovalRule { + category: category(env), + required_approvals, + approvers: approvers.clone(), + approval_timeout_secs: 86_400, + auto_approve: false, + priority: 1, + }, + ); + + Fixture { + env: env.clone(), + registry, + multisig, + multisig_address: multisig_id, + admin, + approvers, + } + } + + /// Registers an asset in the registry owned by the multisig contract. + fn register_governed_asset(&self, seed: u8) -> BytesN<32> { + let id = BytesN::from_array(&self.env, &[seed; 32]); + self.registry.register_asset( + &asset(&self.env, &self.multisig_address, id.clone()), + &self.admin, + ); + id + } + + fn request_transfer(&self, asset_id: &BytesN<32>, to: &Address) -> u64 { + self.multisig.create_transfer_request( + &self.multisig_address, + asset_id, + &category(&self.env), + to, + &BytesN::from_array(&self.env, &[0u8; 32]), + &(self.env.ledger().timestamp() + 100_000), + &None, + ) + } +} + +fn category(env: &Env) -> BytesN<32> { + BytesN::from_array(env, &[CATEGORY_SEED; 32]) +} + +fn asset(env: &Env, owner: &Address, id: BytesN<32>) -> assetsup::asset::Asset { + let timestamp = env.ledger().timestamp(); + assetsup::asset::Asset { + id, + name: String::from_str(env, "High-value asset"), + description: String::from_str(env, "Governed by multisig approval"), + category: String::from_str(env, "Machinery"), + owner: owner.clone(), + registration_timestamp: timestamp, + last_transfer_timestamp: timestamp, + status: assetsup::AssetStatus::Active, + metadata_uri: String::from_str(env, "ipfs://asset"), + purchase_value: 500_000, + custom_attributes: Vec::new(env), + } +} + +// --------------------------------------------------------------------------- +// Flow 1: register -> request -> approve to threshold -> ownership moves +// --------------------------------------------------------------------------- + +#[test] +fn a_fully_approved_transfer_moves_ownership_in_the_registry() { + let env = Env::default(); + let f = Fixture::new(&env, 2, 3); + + let asset_id = f.register_governed_asset(1); + let recipient = Address::generate(&env); + + assert_eq!( + f.registry.get_asset(&asset_id).owner, + f.multisig_address, + "the asset starts owned by the governing contract" + ); + + let request_id = f.request_transfer(&asset_id, &recipient); + + f.multisig + .approve_transfer_request(&f.approvers.get(0).unwrap(), &request_id); + f.multisig + .approve_transfer_request(&f.approvers.get(1).unwrap(), &request_id); + + f.multisig.execute_transfer(&f.admin, &request_id); + + // The assertion the stubbed registry made impossible: ownership actually + // changed in the *other* contract. + assert_eq!( + f.registry.get_asset(&asset_id).owner, + recipient, + "execute_transfer must move ownership in the registry, not just report success" + ); +} + +// --------------------------------------------------------------------------- +// Flow 2: threshold not reached leaves no partial state anywhere +// --------------------------------------------------------------------------- + +#[test] +fn a_transfer_below_the_threshold_leaves_ownership_unchanged() { + let env = Env::default(); + let f = Fixture::new(&env, 3, 3); + + let asset_id = f.register_governed_asset(2); + let recipient = Address::generate(&env); + let request_id = f.request_transfer(&asset_id, &recipient); + + // One approval short. + f.multisig + .approve_transfer_request(&f.approvers.get(0).unwrap(), &request_id); + f.multisig + .approve_transfer_request(&f.approvers.get(1).unwrap(), &request_id); + + let result = f.multisig.try_execute_transfer(&f.admin, &request_id); + assert!(result.is_err(), "execution below the threshold must fail"); + + assert_eq!( + f.registry.get_asset(&asset_id).owner, + f.multisig_address, + "a failed execution must leave ownership untouched" + ); + + // And no partial state in the multisig contract either: the request is + // still pending and can still be completed. + f.multisig + .approve_transfer_request(&f.approvers.get(2).unwrap(), &request_id); + f.multisig.execute_transfer(&f.admin, &request_id); + + assert_eq!(f.registry.get_asset(&asset_id).owner, recipient); +} + +// --------------------------------------------------------------------------- +// Flow 3: an unauthorized actor is rejected at the approval boundary +// --------------------------------------------------------------------------- + +#[test] +fn an_unauthorized_approver_is_rejected_and_cannot_reach_the_threshold() { + let env = Env::default(); + let f = Fixture::new(&env, 2, 2); + + let asset_id = f.register_governed_asset(3); + let recipient = Address::generate(&env); + let request_id = f.request_transfer(&asset_id, &recipient); + + let stranger = Address::generate(&env); + let result = f + .multisig + .try_approve_transfer_request(&stranger, &request_id); + assert!( + result.is_err(), + "an address outside the approver set must be rejected" + ); + + // One legitimate approval is not enough on its own. + f.multisig + .approve_transfer_request(&f.approvers.get(0).unwrap(), &request_id); + + assert!( + f.multisig + .try_execute_transfer(&f.admin, &request_id) + .is_err(), + "the rejected approval must not have counted toward the threshold" + ); + assert_eq!( + f.registry.get_asset(&asset_id).owner, + f.multisig_address, + "ownership must be unchanged" + ); +} + +// --------------------------------------------------------------------------- +// Flow 4: the registry is genuinely consulted, not assumed +// --------------------------------------------------------------------------- + +#[test] +fn a_request_for_an_unregistered_asset_is_rejected() { + // The stubbed asset_exists returned `true` unconditionally, so this case + // silently succeeded against an asset that did not exist. + let env = Env::default(); + let f = Fixture::new(&env, 1, 1); + + let missing = BytesN::from_array(&env, &[99u8; 32]); + let result = f.multisig.try_create_transfer_request( + &f.multisig_address, + &missing, + &category(&env), + &Address::generate(&env), + &BytesN::from_array(&env, &[0u8; 32]), + &(env.ledger().timestamp() + 100_000), + &None, + ); + + assert!( + result.is_err(), + "the registry must be consulted for existence" + ); +} + +#[test] +fn a_request_for_a_retired_asset_is_rejected() { + // asset_is_retired previously returned `false` unconditionally. + let env = Env::default(); + let f = Fixture::new(&env, 1, 1); + + let asset_id = f.register_governed_asset(4); + f.registry.retire_asset(&asset_id, &f.multisig_address); + + let result = f.multisig.try_create_transfer_request( + &f.multisig_address, + &asset_id, + &category(&env), + &Address::generate(&env), + &BytesN::from_array(&env, &[0u8; 32]), + &(env.ledger().timestamp() + 100_000), + &None, + ); + + assert!(result.is_err(), "a retired asset must not be transferable"); +} + +#[test] +fn a_non_owner_cannot_raise_a_transfer_request() { + // The ownership check was skipped entirely while the registry was stubbed, + // so anyone could raise a request against any asset. + let env = Env::default(); + let f = Fixture::new(&env, 1, 1); + + let asset_id = f.register_governed_asset(5); + let stranger = Address::generate(&env); + + let result = f.multisig.try_create_transfer_request( + &stranger, + &asset_id, + &category(&env), + &Address::generate(&env), + &BytesN::from_array(&env, &[0u8; 32]), + &(env.ledger().timestamp() + 100_000), + &None, + ); + + assert!( + result.is_err(), + "only the asset's owner or the admin may raise a request" + ); +} + +// --------------------------------------------------------------------------- +// Flow 5: the audit trail spans both contracts +// --------------------------------------------------------------------------- + +#[test] +fn a_completed_transfer_is_visible_in_both_contracts() { + let env = Env::default(); + let f = Fixture::new(&env, 1, 1); + + let asset_id = f.register_governed_asset(6); + let recipient = Address::generate(&env); + let request_id = f.request_transfer(&asset_id, &recipient); + + f.multisig + .approve_transfer_request(&f.approvers.get(0).unwrap(), &request_id); + f.multisig.execute_transfer(&f.admin, &request_id); + + // Registry side: the new owner, and the asset listed under them. + assert_eq!(f.registry.get_asset(&asset_id).owner, recipient); + assert_eq!(f.registry.get_assets_by_owner(&recipient).len(), 1); + + // Multisig side: the request records this asset in its history. + let history = f.multisig.get_asset_history(&asset_id); + assert_eq!(history.len(), 1); + assert_eq!(history.get(0).unwrap(), request_id); +} + +#[test] +fn two_assets_are_governed_independently() { + let env = Env::default(); + let f = Fixture::new(&env, 1, 1); + + let first = f.register_governed_asset(7); + let second = f.register_governed_asset(8); + let recipient = Address::generate(&env); + + let request_id = f.request_transfer(&first, &recipient); + f.multisig + .approve_transfer_request(&f.approvers.get(0).unwrap(), &request_id); + f.multisig.execute_transfer(&f.admin, &request_id); + + assert_eq!(f.registry.get_asset(&first).owner, recipient); + assert_eq!( + f.registry.get_asset(&second).owner, + f.multisig_address, + "an unrelated asset must be untouched" + ); +} diff --git a/contracts/multisig-transfer/src/registry.rs b/contracts/multisig-transfer/src/registry.rs index 10e493eb6..d4a5430af 100644 --- a/contracts/multisig-transfer/src/registry.rs +++ b/contracts/multisig-transfer/src/registry.rs @@ -1,23 +1,55 @@ -use soroban_sdk::{Address, BytesN, Env}; +//! Cross-contract calls into the asset registry. +//! +//! These were placeholders: `asset_exists` returned `true` unconditionally, +//! `asset_is_retired` returned `false`, `get_owner` always errored, and +//! `transfer_owner` did nothing and returned `Ok`. That last one meant +//! `execute_transfer` reported success without ever moving ownership — the +//! entire purpose of this contract was a no-op. +//! +//! They now invoke the registry for real, using +//! [`Env::invoke_contract`] rather than importing the registry crate, so the +//! two contracts stay decoupled and either can be redeployed independently. +//! +//! ## Expected registry interface +//! +//! The contract at the stored `AssetRegistry` address must expose: +//! +//! ```text +//! check_asset_exists(asset_id: BytesN<32>) -> bool +//! get_asset(asset_id: BytesN<32>) -> Asset // with .owner and .status +//! transfer_asset_ownership(asset_id: BytesN<32>, new_owner: Address, caller: Address) +//! ``` +//! +//! `assetsup` satisfies this. +//! +//! ## Authorization +//! +//! `transfer_asset_ownership` authenticates its `caller` and requires it to be +//! the asset's current owner. This contract passes its **own** address as +//! `caller`, which works because a contract implicitly authorizes calls it +//! makes itself. The consequence is a deployment requirement: **assets governed +//! by this workflow must be owned by this contract's address**, which then +//! releases them once the approval threshold is met. An asset owned by an +//! ordinary account cannot be moved by this contract, and the transfer will +//! fail at the registry rather than silently succeeding. + +use soroban_sdk::{Address, BytesN, Env, IntoVal, Symbol, Val, Vec}; use crate::errors::MultiSigError; -/// NOTE: -/// Replace these methods with your real Asset Registry contract interface. -/// This contract assumes registry supports: -/// - asset_exists(asset_id) -> bool -/// - is_retired(asset_id) -> bool -/// - get_owner(asset_id) -> Address -/// - transfer(asset_id, new_owner) +/// Status values mirrored from the registry's `AssetStatus`. Kept as a local +/// constant rather than a shared type so the crates stay decoupled; the +/// registry's discriminant order is part of the interface contract above. +const STATUS_RETIRED: u32 = 2; + pub fn asset_exists( e: &Env, registry: &Address, asset_id: &BytesN<32>, ) -> Result { - // implement using generated client for registry - // registry_client.asset_exists(asset_id) - let _ = (e, registry, asset_id); - Ok(true) // placeholder + let args: Vec = (asset_id.clone(),).into_val(e); + let exists: bool = e.invoke_contract(registry, &Symbol::new(e, "check_asset_exists"), args); + Ok(exists) } pub fn asset_is_retired( @@ -25,27 +57,82 @@ pub fn asset_is_retired( registry: &Address, asset_id: &BytesN<32>, ) -> Result { - let _ = (e, registry, asset_id); - Ok(false) // placeholder + let info = asset_info(e, registry, asset_id)?; + Ok(info.1 == STATUS_RETIRED) } -#[allow(dead_code)] pub fn get_owner( e: &Env, registry: &Address, asset_id: &BytesN<32>, ) -> Result { - let _ = (e, registry, asset_id); - Err(MultiSigError::RegistryCallFailed) + Ok(asset_info(e, registry, asset_id)?.0) } +/// Moves ownership in the registry. +/// +/// Passes this contract's own address as `caller`; see the module docs for why +/// that requires governed assets to be owned by this contract. pub fn transfer_owner( e: &Env, registry: &Address, asset_id: &BytesN<32>, new_owner: &Address, ) -> Result<(), MultiSigError> { - let _ = (e, registry, asset_id, new_owner); - // registry_client.transfer(asset_id, new_owner) + let caller = e.current_contract_address(); + let args: Vec = (asset_id.clone(), new_owner.clone(), caller).into_val(e); + + e.invoke_contract::<()>(registry, &Symbol::new(e, "transfer_asset_ownership"), args); + Ok(()) } + +/// Reads `(owner, status)` from the registry's asset info. +/// +/// `get_asset_info` returns a struct whose first two fields this contract +/// cares about. Decoding it positionally keeps the crates decoupled at the +/// cost of depending on field order, which is why the expected interface is +/// spelled out in the module docs. +fn asset_info( + e: &Env, + registry: &Address, + asset_id: &BytesN<32>, +) -> Result<(Address, u32), MultiSigError> { + if !asset_exists(e, registry, asset_id)? { + return Err(MultiSigError::AssetNotFound); + } + + let args: Vec = (asset_id.clone(),).into_val(e); + let info: AssetInfo = e.invoke_contract(registry, &Symbol::new(e, "get_asset_info"), args); + + Ok((info.owner, status_code(&info.status))) +} + +fn status_code(status: &AssetStatus) -> u32 { + match status { + AssetStatus::Active => 0, + AssetStatus::Transferred => 1, + AssetStatus::Retired => STATUS_RETIRED, + } +} + +/// Mirror of the registry's `AssetInfo`, declared locally so this crate does +/// not depend on the registry crate. The field order and names must match. +#[soroban_sdk::contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct AssetInfo { + pub id: BytesN<32>, + pub name: soroban_sdk::String, + pub category: soroban_sdk::String, + pub owner: Address, + pub status: AssetStatus, +} + +/// Mirror of the registry's `AssetStatus`. Variant order is significant. +#[soroban_sdk::contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum AssetStatus { + Active, + Transferred, + Retired, +} diff --git a/contracts/multisig-wallet/Cargo.toml b/contracts/multisig-wallet/Cargo.toml index 70038ef84..021c15f6a 100644 --- a/contracts/multisig-wallet/Cargo.toml +++ b/contracts/multisig-wallet/Cargo.toml @@ -12,3 +12,5 @@ soroban-sdk = { workspace = true } [dev-dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } +# Property-based tests for threshold and confirmation invariants ([SC-50]). +proptest = "1" diff --git a/contracts/multisig-wallet/src/lib.rs b/contracts/multisig-wallet/src/lib.rs index ef17fe6c8..4f2f0cdb1 100644 --- a/contracts/multisig-wallet/src/lib.rs +++ b/contracts/multisig-wallet/src/lib.rs @@ -33,6 +33,7 @@ mod errors; #[cfg(test)] mod event_tests; pub mod events; +mod proptests; #[cfg(test)] mod tests; mod types; diff --git a/contracts/multisig-wallet/src/proptests.rs b/contracts/multisig-wallet/src/proptests.rs new file mode 100644 index 000000000..7a3f9f9ee --- /dev/null +++ b/contracts/multisig-wallet/src/proptests.rs @@ -0,0 +1,236 @@ +//! Property-based tests for threshold and confirmation invariants ([SC-50]). +//! +//! The example-based tests check specific sequences. These check that an +//! invariant holds across *any* generated sequence of owner additions, +//! removals and confirmations — which is where threshold logic tends to break, +//! because the interesting cases are combinations nobody thought to write down. +//! +//! Case counts are kept modest so the suite stays CI-appropriate; each case +//! spins up a fresh Soroban `Env`, which is not free. +#![cfg(test)] + +extern crate std; + +use proptest::prelude::*; +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, Env, Symbol, Vec}; + +use crate::types::{ProposalStatus, TransactionType}; +use crate::{MultisigWallet, MultisigWalletClient}; + +/// Builds an `owner_count`-owner wallet with the given threshold. +fn wallet( + env: &Env, + owner_count: usize, + threshold: u32, +) -> (MultisigWalletClient<'_>, Vec
) { + let contract_id = env.register(MultisigWallet, ()); + let client = MultisigWalletClient::new(env, &contract_id); + let admin = Address::generate(env); + + let mut owners = Vec::new(env); + for _ in 0..owner_count { + owners.push_back(Address::generate(env)); + } + + env.mock_all_auths(); + client.initialize(&admin, &owners, &threshold); + (client, owners) +} + +fn submit(env: &Env, client: &MultisigWalletClient, initiator: &Address) -> u64 { + client.submit_transaction( + initiator, + &TransactionType::Routine, + &Address::generate(env), + &Symbol::new(env, "noop"), + &Vec::new(env), + &3600, + &0, + ) +} + +proptest! { + #![proptest_config(ProptestConfig::with_cases(24))] + + /// A transaction's confirmation count must always equal the number of + /// *distinct current owners* who confirmed it — never more, however the + /// confirmations are ordered or repeated. + #[test] + fn confirmation_count_equals_distinct_confirming_owners( + owner_count in 2usize..6, + // Indices into the owner set, with deliberate repeats so duplicate + // confirmations are exercised. + confirmations in prop::collection::vec(0usize..6, 0..12), + ) { + let threshold = 1 + (owner_count as u32) / 2; + let env = Env::default(); + let (client, owners) = wallet(&env, owner_count, threshold); + + let tx_id = submit(&env, &client, &owners.get(0).unwrap()); + + let mut distinct = std::collections::BTreeSet::new(); + let mut executed = false; + + for idx in confirmations { + if idx >= owner_count { + continue; + } + let owner = owners.get(idx as u32).unwrap(); + + // Once the threshold is reached the transaction auto-executes and + // further confirmations are rejected; stop tracking there. + if client.try_confirm_transaction(&owner, &tx_id).is_ok() { + distinct.insert(idx); + } + + let tx = client.get_transaction(&tx_id).unwrap(); + if tx.confirmations_count >= threshold { + executed = true; + } + + if !executed { + prop_assert_eq!( + tx.confirmations_count as usize, + distinct.len(), + "count must equal the number of distinct owners who confirmed" + ); + } + } + } + + /// A duplicate confirmation from the same owner never increases the count. + #[test] + fn a_repeated_confirmation_never_raises_the_count( + owner_count in 2usize..6, + repeats in 1usize..5, + ) { + let env = Env::default(); + // Threshold above 1 so the first confirmation does not auto-execute. + let (client, owners) = wallet(&env, owner_count, owner_count as u32); + let tx_id = submit(&env, &client, &owners.get(0).unwrap()); + let confirmer = owners.get(0).unwrap(); + + client.confirm_transaction(&confirmer, &tx_id); + let after_first = client.get_transaction(&tx_id).unwrap().confirmations_count; + + for _ in 0..repeats { + let _ = client.try_confirm_transaction(&confirmer, &tx_id); + prop_assert_eq!( + client.get_transaction(&tx_id).unwrap().confirmations_count, + after_first, + "a repeated confirmation must not move the count" + ); + } + } + + /// A proposal executes only when at least `threshold` distinct owners have + /// confirmed it — never on fewer. + #[test] + fn a_proposal_never_executes_below_the_threshold( + owner_count in 3usize..6, + confirming in 1usize..3, + ) { + let env = Env::default(); + let threshold = owner_count as u32; + let (client, owners) = wallet(&env, owner_count, threshold); + + let candidate = Address::generate(&env); + let proposal_id = client.propose_add_owner(&owners.get(0).unwrap(), &candidate); + + // Confirm with strictly fewer owners than the threshold. + let confirming = confirming.min(owner_count - 1); + for i in 0..confirming { + client.confirm_proposal(&owners.get(i as u32).unwrap(), &proposal_id); + } + + let proposal = client.get_proposal(&proposal_id).unwrap(); + prop_assert_eq!( + proposal.status, + ProposalStatus::Pending, + "a proposal below its threshold must stay pending" + ); + prop_assert!( + !client.get_owners().contains(candidate), + "the owner must not have been added" + ); + prop_assert!( + client.try_execute_proposal(&proposal_id).is_err(), + "explicit execution below the threshold must be rejected" + ); + } + + /// The threshold invariant holds after any accepted governance change: + /// `1 <= threshold <= owners.len()`, and there are always at least two + /// owners. + #[test] + fn threshold_stays_within_the_owner_count( + owner_count in 2usize..6, + requested_threshold in 0u32..10, + ) { + let env = Env::default(); + let (client, owners) = wallet(&env, owner_count, 1); + + // A threshold of 1 means one confirmation executes the proposal. + let proposer = owners.get(0).unwrap(); + let result = client.try_propose_change_threshold(&proposer, &requested_threshold); + + if result.is_ok() { + let proposal_id = result.unwrap().unwrap(); + client.confirm_proposal(&proposer, &proposal_id); + } + + let threshold = client.get_threshold(); + let count = client.get_owners().len(); + + prop_assert!(threshold >= 1, "threshold must never drop to zero"); + prop_assert!( + threshold <= count, + "threshold must never exceed the owner count" + ); + prop_assert!(count >= 2, "a multisig must keep at least two owners"); + } + + /// Removing owners can never leave the wallet with a threshold its + /// remaining owners could not reach. + #[test] + fn removals_never_strand_the_threshold( + owner_count in 3usize..6, + removals in 1usize..4, + ) { + let env = Env::default(); + let threshold = 2u32; + let (client, owners) = wallet(&env, owner_count, threshold); + let proposer = owners.get(0).unwrap(); + + for i in 0..removals { + let victim_index = owner_count - 1 - (i % owner_count); + if victim_index == 0 { + continue; + } + let victim = owners.get(victim_index as u32).unwrap(); + if !client.get_owners().contains(victim.clone()) { + continue; + } + + if let Ok(Ok(proposal_id)) = client.try_propose_remove_owner(&proposer, &victim) { + // Threshold is 2, so two distinct owners must confirm. + let current = client.get_owners(); + client.confirm_proposal(¤t.get(0).unwrap(), &proposal_id); + if current.len() > 1 { + let _ = client.try_confirm_proposal(¤t.get(1).unwrap(), &proposal_id); + } + } + + let remaining = client.get_owners().len(); + prop_assert!( + remaining >= 2, + "removal must never drop the wallet below two owners" + ); + prop_assert!( + client.get_threshold() <= remaining, + "removal must never leave an unreachable threshold" + ); + } + } +} diff --git a/contracts/scripts/check-wasm-size.sh b/contracts/scripts/check-wasm-size.sh new file mode 100755 index 000000000..600ef5fb4 --- /dev/null +++ b/contracts/scripts/check-wasm-size.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash +# +# Reports each contract's release WASM size and fails if any exceeds its +# budget ([SC-52]). +# +# Soroban enforces a ledger limit on deployed contract size and charges by +# resource usage. Without tracking, growth is invisible until a deploy fails — +# which is the worst possible moment to discover it. +# +# Usage: +# cargo build --workspace --target wasm32-unknown-unknown --release +# ./scripts/check-wasm-size.sh +# +# ./scripts/check-wasm-size.sh --json # machine-readable, for PR diffing +# ./scripts/check-wasm-size.sh --baseline # rewrite the recorded baselines +# +# Budgets are set roughly 25% above the measured size, so ordinary feature work +# does not trip them but a sudden jump does. Raising a budget should be a +# deliberate, reviewed decision — say why in the PR. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +CONTRACTS_DIR="$(dirname "$SCRIPT_DIR")" +cd "$CONTRACTS_DIR" + +WASM_DIR="target/wasm32-unknown-unknown/release" +BASELINE_FILE="scripts/wasm-size-baseline.txt" + +MODE="report" +case "${1:-}" in + --json) MODE="json" ;; + --baseline) MODE="baseline" ;; + "") ;; + *) echo "error: unknown argument '$1'" >&2; exit 2 ;; +esac + +# Per-contract budget in bytes. Keyed by the artifact name, which uses +# underscores even where the crate name uses hyphens. +budget_for() { + case "$1" in + assetsup) echo 215000 ;; + contrib) echo 100000 ;; + multisig_wallet) echo 95000 ;; + asset_maintenance) echo 85000 ;; + multisig_transfer) echo 85000 ;; + # An unbudgeted contract fails rather than passing silently: a new + # deployable contract must get a reviewed budget. + *) echo 0 ;; + esac +} + +if [[ ! -d "$WASM_DIR" ]]; then + echo "error: $WASM_DIR not found." >&2 + echo " Run: cargo build --workspace --target wasm32-unknown-unknown --release" >&2 + exit 1 +fi + +shopt -s nullglob +wasm_files=("$WASM_DIR"/*.wasm) +shopt -u nullglob + +if [[ ${#wasm_files[@]} -eq 0 ]]; then + echo "error: no .wasm artifacts in $WASM_DIR" >&2 + exit 1 +fi + +# ---------------------------------------------------------------- baseline + +if [[ "$MODE" == "baseline" ]]; then + : > "$BASELINE_FILE" + { + echo "# Recorded release WASM sizes in bytes." + echo "# Regenerate with: ./scripts/check-wasm-size.sh --baseline" + for wasm in "${wasm_files[@]}"; do + name=$(basename "$wasm" .wasm) + printf '%s %s\n' "$name" "$(wc -c < "$wasm" | tr -d ' ')" + done + } >> "$BASELINE_FILE" + echo "Wrote $BASELINE_FILE" + exit 0 +fi + +baseline_for() { + [[ -f "$BASELINE_FILE" ]] || { echo ""; return; } + awk -v n="$1" '$1 == n { print $2 }' "$BASELINE_FILE" +} + +# ---------------------------------------------------------------- json + +if [[ "$MODE" == "json" ]]; then + printf '{\n' + last=$(( ${#wasm_files[@]} - 1 )) + for i in "${!wasm_files[@]}"; do + name=$(basename "${wasm_files[$i]}" .wasm) + size=$(wc -c < "${wasm_files[$i]}" | tr -d ' ') + sep=","; [[ "$i" -eq "$last" ]] && sep="" + printf ' "%s": %s%s\n' "$name" "$size" "$sep" + done + printf '}\n' + exit 0 +fi + +# ---------------------------------------------------------------- report + +failed=0 +total=0 + +printf '%-22s %10s %10s %10s %8s\n' "contract" "size" "baseline" "budget" "result" +printf -- '---------------------------------------------------------------\n' + +for wasm in "${wasm_files[@]}"; do + name=$(basename "$wasm" .wasm) + size=$(wc -c < "$wasm" | tr -d ' ') + budget="$(budget_for "$name")" + baseline="$(baseline_for "$name")" + total=$(( total + size )) + + if [[ "$budget" -eq 0 ]]; then + printf '%-22s %10s %10s %10s %8s\n' "$name" "$size" "${baseline:--}" "none" "FAIL" + echo " ^ no budget configured. Add one to budget_for() in this script." >&2 + failed=1 + continue + fi + + # Show the delta against the recorded baseline so growth is visible even + # while still inside budget. + delta="" + if [[ -n "$baseline" && "$baseline" -gt 0 ]]; then + diff=$(( size - baseline )) + if [[ "$diff" -gt 0 ]]; then + delta="+${diff}" + elif [[ "$diff" -lt 0 ]]; then + delta="${diff}" + else + delta="0" + fi + fi + + if [[ "$size" -gt "$budget" ]]; then + printf '%-22s %10s %10s %10s %8s\n' "$name" "$size" "${baseline:--}" "$budget" "FAIL" + over=$(( size - budget )) + echo " ^ over budget by ${over} bytes" >&2 + failed=1 + else + headroom=$(( budget - size )) + printf '%-22s %10s %10s %10s %8s\n' "$name" "$size" "${baseline:--}" "$budget" "ok" + if [[ -n "$delta" && "$delta" != "0" ]]; then + printf ' delta vs baseline: %s bytes (%s headroom)\n' "$delta" "$headroom" + fi + fi +done + +printf -- '---------------------------------------------------------------\n' +printf '%-22s %10s\n' "total" "$total" + +if [[ "$failed" -ne 0 ]]; then + echo >&2 + echo "WASM size budget exceeded. Either reduce the artifact, or raise the" >&2 + echo "budget in scripts/check-wasm-size.sh with a reason in the PR." >&2 + exit 1 +fi + +echo +echo "All contracts within budget." diff --git a/contracts/scripts/wasm-size-baseline.txt b/contracts/scripts/wasm-size-baseline.txt new file mode 100644 index 000000000..5486e99e3 --- /dev/null +++ b/contracts/scripts/wasm-size-baseline.txt @@ -0,0 +1,7 @@ +# Recorded release WASM sizes in bytes. +# Regenerate with: ./scripts/check-wasm-size.sh --baseline +asset_maintenance 65756 +assetsup 168165 +contrib 79282 +multisig_transfer 64938 +multisig_wallet 74210 From 8b7554b96c3123c85be5052cc2bd2f96f6fe9c68 Mon Sep 17 00:00:00 2001 From: TomikeDS <203535757+TomikeDS@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:01:07 +0100 Subject: [PATCH 2/3] Make the WASM size delta job work for fork pull requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job failed with "Resource not accessible by integration". A pull_request event from a fork gets a read-only GITHUB_TOKEN whatever the permissions block declares — that is a GitHub security boundary, not a misconfiguration — so the comment API call could never succeed for exactly the PRs this repository receives. Reworked so the delta is always produced and always visible: - The table is written to the job summary, which needs no token and shows up in the checks UI for every PR including forks. That is now the primary output. - The PR comment is best-effort: skipped entirely for fork PRs, and marked continue-on-error so a token or API problem cannot fail the build over a convenience feature. Switching to pull_request_target would restore write access, but it runs with a privileged token against the PR's own code, which is not a trade worth making for a size report. The table is built in Python rather than inline JS so it renders identically in the summary and the comment. Verified against grown, shrunk and newly added contracts. --- .github/workflows/CI.yaml | 142 +++++++++++++++++++++++++++++++++----- 1 file changed, 126 insertions(+), 16 deletions(-) diff --git a/.github/workflows/CI.yaml b/.github/workflows/CI.yaml index 14a57ce92..dbcadc21a 100644 --- a/.github/workflows/CI.yaml +++ b/.github/workflows/CI.yaml @@ -132,25 +132,13 @@ jobs: restore-keys: | ${{ runner.os }}-cargo-wasm- ${{ runner.os }}-cargo- - # Every crate in the workspace is a deployable contract: each declares a - # `cdylib` crate-type and a `#[contract]` entrypoint. Building the whole - # workspace for wasm32 therefore fails the job if any contract cannot be - # deployed, which native compilation alone does not guarantee. + # Every crate declares a cdylib crate-type and a #[contract] entrypoint, + # so all five are deployable and all five must be size-checked. - name: Build all contracts to WASM run: cargo build --workspace --target wasm32-unknown-unknown --release --verbose - - name: Report WASM artifact sizes - run: | - echo "### WASM artifact sizes" >> "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" - echo "| Contract | Size (bytes) |" >> "$GITHUB_STEP_SUMMARY" - echo "|---|---:|" >> "$GITHUB_STEP_SUMMARY" - for wasm in target/wasm32-unknown-unknown/release/*.wasm; do - name=$(basename "$wasm" .wasm) - size=$(wc -c < "$wasm" | tr -d ' ') - printf '| %s | %s |\n' "$name" "$size" >> "$GITHUB_STEP_SUMMARY" - done - cat "$GITHUB_STEP_SUMMARY" + - name: Check WASM size budgets + run: ./scripts/check-wasm-size.sh | tee "$GITHUB_STEP_SUMMARY" - name: Upload WASM artifacts uses: actions/upload-artifact@v4 @@ -160,6 +148,128 @@ jobs: if-no-files-found: error retention-days: 14 + wasm-size-delta: + name: WASM Size Delta + runs-on: ubuntu-latest + # Only meaningful on a pull request, where there is a base to compare to. + if: github.event_name == 'pull_request' + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/checkout@v4 + - name: Read pinned toolchain + id: toolchain + run: echo "channel=$(grep -m1 '^channel' rust-toolchain.toml | cut -d'"' -f2)" >> "$GITHUB_OUTPUT" + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ steps.toolchain.outputs.channel }} + targets: wasm32-unknown-unknown + - name: Cache cargo registry + uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + contracts/target + key: ${{ runner.os }}-cargo-wasm-${{ hashFiles('contracts/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo-wasm- + ${{ runner.os }}-cargo- + + - name: Measure this branch + run: | + cargo build --workspace --target wasm32-unknown-unknown --release + ./scripts/check-wasm-size.sh --json > /tmp/head-sizes.json + cat /tmp/head-sizes.json + + - name: Measure the base branch + run: | + git stash --include-untracked || true + git fetch origin "${{ github.base_ref }}" --depth=1 + git checkout "origin/${{ github.base_ref }}" -- . + cargo build --workspace --target wasm32-unknown-unknown --release + ./scripts/check-wasm-size.sh --json > /tmp/base-sizes.json || echo '{}' > /tmp/base-sizes.json + cat /tmp/base-sizes.json + + - name: Build the delta table + id: delta + run: | + python3 - <<'PY' > /tmp/delta.md + import json + + with open('/tmp/head-sizes.json') as f: + head = json.load(f) + try: + with open('/tmp/base-sizes.json') as f: + base = json.load(f) + except Exception: + base = {} + + rows = [] + for name in sorted(head): + now = head[name] + before = base.get(name) + if before is None: + rows.append(f"| `{name}` | — | {now} | new |") + continue + diff = now - before + pct = 0 if before == 0 else (diff / before) * 100 + sign = "+" if diff > 0 else "" + marker = " :warning:" if diff > 0 else (" :white_check_mark:" if diff < 0 else "") + rows.append(f"| `{name}` | {before} | {now} | {sign}{diff} ({sign}{pct:.2f}%){marker} |") + + print("### WASM size delta") + print() + print("| Contract | Base | This PR | Change |") + print("|---|---:|---:|---:|") + print("\n".join(rows)) + print() + print("_Sizes in bytes, before `stellar contract optimize`. " + "Budgets are in `contracts/scripts/check-wasm-size.sh`._") + PY + cat /tmp/delta.md >> "$GITHUB_STEP_SUMMARY" + cat /tmp/delta.md + + # A pull_request event from a fork gets a read-only GITHUB_TOKEN whatever + # the permissions block says, so commenting is best-effort. The delta is + # always in the job summary above; this only adds the convenience of + # having it inline on the PR when the token allows it. + - name: Comment the per-contract delta + if: github.event.pull_request.head.repo.full_name == github.repository + continue-on-error: true + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const body = fs.readFileSync('/tmp/delta.md', 'utf8'); + + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + const existing = comments.find( + (c) => c.user.type === 'Bot' && c.body.startsWith('### WASM size delta') + ); + + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + } + # ───────────────────────────────────────────── # BACKEND — NestJS # ───────────────────────────────────────────── From be0094430f7966872ae390ad5d8f881228196d00 Mon Sep 17 00:00:00 2001 From: portable Date: Mon, 27 Jul 2026 18:08:44 +0100 Subject: [PATCH 3/3] fix: configure Jest for JSX transform, remove vitest imports, add @testing-library/react --- .../AssetHistoryComponent.spec.tsx | 1 - .../AuthFlow/AuthFlowComponent.spec.tsx | 1 - .../LoadingSkeletonsComponent.spec.tsx | 1 - .../LocationsManagementComponent.spec.tsx | 1 - frontend/jest.config.js | 19 ++ frontend/package-lock.json | 240 ++++++++++++++++++ frontend/package.json | 2 + 7 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 frontend/jest.config.js diff --git a/frontend/features/AssetHistory/AssetHistoryComponent.spec.tsx b/frontend/features/AssetHistory/AssetHistoryComponent.spec.tsx index ba93b537c..76bf6aa42 100644 --- a/frontend/features/AssetHistory/AssetHistoryComponent.spec.tsx +++ b/frontend/features/AssetHistory/AssetHistoryComponent.spec.tsx @@ -1,5 +1,4 @@ import React from "react"; -import { describe, it, expect } from "vitest"; import { render, screen } from "@testing-library/react"; import { AssetHistoryComponent } from "./AssetHistoryComponent"; describe("AssetHistoryComponent", () => { diff --git a/frontend/features/AuthFlow/AuthFlowComponent.spec.tsx b/frontend/features/AuthFlow/AuthFlowComponent.spec.tsx index fe8f4b573..66528ad89 100644 --- a/frontend/features/AuthFlow/AuthFlowComponent.spec.tsx +++ b/frontend/features/AuthFlow/AuthFlowComponent.spec.tsx @@ -1,5 +1,4 @@ import React from "react"; -import { describe, it, expect } from "vitest"; import { render, screen } from "@testing-library/react"; import { AuthFlowComponent } from "./AuthFlowComponent"; describe("AuthFlowComponent", () => { diff --git a/frontend/features/LoadingSkeletons/LoadingSkeletonsComponent.spec.tsx b/frontend/features/LoadingSkeletons/LoadingSkeletonsComponent.spec.tsx index 40318fdde..6307d4ba1 100644 --- a/frontend/features/LoadingSkeletons/LoadingSkeletonsComponent.spec.tsx +++ b/frontend/features/LoadingSkeletons/LoadingSkeletonsComponent.spec.tsx @@ -1,5 +1,4 @@ import React from "react"; -import { describe, it, expect } from "vitest"; import { render } from "@testing-library/react"; import { LoadingSkeletonsComponent } from "./LoadingSkeletonsComponent"; describe("LoadingSkeletonsComponent", () => { diff --git a/frontend/features/LocationsManagement/LocationsManagementComponent.spec.tsx b/frontend/features/LocationsManagement/LocationsManagementComponent.spec.tsx index 138b8ebf1..217921fa7 100644 --- a/frontend/features/LocationsManagement/LocationsManagementComponent.spec.tsx +++ b/frontend/features/LocationsManagement/LocationsManagementComponent.spec.tsx @@ -1,5 +1,4 @@ import React from "react"; -import { describe, it, expect } from "vitest"; import { render, screen } from "@testing-library/react"; import { LocationsManagementComponent } from "./LocationsManagementComponent"; describe("LocationsManagementComponent", () => { diff --git a/frontend/jest.config.js b/frontend/jest.config.js new file mode 100644 index 000000000..80dd39e33 --- /dev/null +++ b/frontend/jest.config.js @@ -0,0 +1,19 @@ +/** @type {import('jest').Config} */ +const config = { + testEnvironment: "jsdom", + transform: { + "^.+\\.tsx?$": [ + "ts-jest", + { + tsconfig: { + jsx: "react-jsx", + }, + }, + ], + }, + moduleNameMapper: { + "^@/(.*)$": "/$1", + }, +}; + +module.exports = config; diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2a376a595..3f98ad6ad 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -37,6 +37,8 @@ "devDependencies": { "@eslint/eslintrc": "^3", "@tailwindcss/postcss": "^4", + "@testing-library/jest-dom": "^7.0.0", + "@testing-library/react": "^16.3.2", "@types/jest": "^30.0.0", "@types/node": "^20", "@types/qrcode": "^1.5.6", @@ -54,6 +56,13 @@ "typescript": "^5" } }, + "node_modules/@adobe/css-tools": { + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/@adobe/css-tools/-/css-tools-4.5.0.tgz", + "integrity": "sha512-6OzddxPio9UiWTCemp4N8cYLV2ZN1ncRnV1cVGtve7dhPOtRkleRyx32GQCYSwDYgaHU3USMm84tNsvKzRCa1Q==", + "dev": true, + "license": "MIT" + }, "node_modules/@alloc/quick-lru": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/@alloc/quick-lru/-/quick-lru-5.2.0.tgz", @@ -2747,6 +2756,145 @@ "url": "https://github.com/sponsors/tannerlinsley" } }, + "node_modules/@testing-library/dom": { + "version": "10.4.1", + "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-10.4.1.tgz", + "integrity": "sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==", + "dev": true, + "license": "MIT", + "peer": true, + "dependencies": { + "@babel/code-frame": "^7.10.4", + "@babel/runtime": "^7.12.5", + "@types/aria-query": "^5.0.1", + "aria-query": "5.3.0", + "dom-accessibility-api": "^0.5.9", + "lz-string": "^1.5.0", + "picocolors": "1.1.1", + "pretty-format": "^27.0.2" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@testing-library/dom/node_modules/ansi-regex": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz", + "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", + "dev": true, + "license": "MIT", + "peer": true, + "engines": { + "node": ">=8" + } + }, + "node_modules/@testing-library/dom/node_modules/ansi-styles": { + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-5.2.0.tgz", + "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", + "dev": true, + "license": "MIT", + "peer": true, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/chalk/ansi-styles?sponsor=1" + } + }, + "node_modules/@testing-library/dom/node_modules/aria-query": { + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/aria-query/-/aria-query-5.3.0.tgz", + "integrity": "sha512-b0P0sZPKtyu8HkeRAfCq0IfURZK+SuwMjY1UXGBU27wpAiTwQAIlq56IbIO+ytk/JjS1fMR14ee5WBBfKi5J6A==", + "dev": true, + "license": "Apache-2.0", + "peer": true, + "dependencies": { + "dequal": "^2.0.3" + } + }, + "node_modules/@testing-library/dom/node_modules/pretty-format": { + "version": "27.5.1", + "resolved": "https://registry.npmjs.org/pretty-format/-/pretty-format-27.5.1.tgz", + "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", + "dev": true, + "license": "MIT", + "peer": true, + "dependencies": { + "ansi-regex": "^5.0.1", + "ansi-styles": "^5.0.0", + "react-is": "^17.0.1" + }, + "engines": { + "node": "^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0" + } + }, + "node_modules/@testing-library/dom/node_modules/react-is": { + "version": "17.0.2", + "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", + "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", + "dev": true, + "license": "MIT", + "peer": true + }, + "node_modules/@testing-library/jest-dom": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/@testing-library/jest-dom/-/jest-dom-7.0.0.tgz", + "integrity": "sha512-HKAH9C6mBo5yBG6yRO5i43L2iisencAo5z+o5P/saHUoY+miC5ivXRxHBJcFyB5ypPNxHJdK3BoF/3O4DIptMg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@adobe/css-tools": "^4.4.0", + "aria-query": "^5.0.0", + "css.escape": "^1.5.1", + "dom-accessibility-api": "^0.6.3", + "picocolors": "^1.1.1", + "redent": "^3.0.0" + }, + "engines": { + "node": ">=22", + "npm": ">=6", + "yarn": ">=1" + }, + "peerDependencies": { + "@testing-library/dom": ">=10 <11" + } + }, + "node_modules/@testing-library/jest-dom/node_modules/dom-accessibility-api": { + "version": "0.6.3", + "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.6.3.tgz", + "integrity": "sha512-7ZgogeTnjuHbo+ct10G9Ffp0mif17idi0IyWNVA/wcwcm7NPOD/WEHVP3n7n3MhXqxoIYm8d6MuZohYWIZ4T3w==", + "dev": true, + "license": "MIT" + }, + "node_modules/@testing-library/react": { + "version": "16.3.2", + "resolved": "https://registry.npmjs.org/@testing-library/react/-/react-16.3.2.tgz", + "integrity": "sha512-XU5/SytQM+ykqMnAnvB2umaJNIOsLF3PVv//1Ew4CTcpz0/BRyy/af40qqrt7SjKpDdT1saBMc42CUok5gaw+g==", + "dev": true, + "license": "MIT", + "dependencies": { + "@babel/runtime": "^7.12.5" + }, + "engines": { + "node": ">=18" + }, + "peerDependencies": { + "@testing-library/dom": "^10.0.0", + "@types/react": "^18.0.0 || ^19.0.0", + "@types/react-dom": "^18.0.0 || ^19.0.0", + "react": "^18.0.0 || ^19.0.0", + "react-dom": "^18.0.0 || ^19.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "@types/react-dom": { + "optional": true + } + } + }, "node_modules/@tybys/wasm-util": { "version": "0.10.1", "resolved": "https://registry.npmjs.org/@tybys/wasm-util/-/wasm-util-0.10.1.tgz", @@ -2758,6 +2906,14 @@ "tslib": "^2.4.0" } }, + "node_modules/@types/aria-query": { + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", + "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", + "dev": true, + "license": "MIT", + "peer": true + }, "node_modules/@types/babel__core": { "version": "7.20.5", "resolved": "https://registry.npmjs.org/@types/babel__core/-/babel__core-7.20.5.tgz", @@ -4572,6 +4728,13 @@ "utrie": "^1.0.2" } }, + "node_modules/css.escape": { + "version": "1.5.1", + "resolved": "https://registry.npmjs.org/css.escape/-/css.escape-1.5.1.tgz", + "integrity": "sha512-YUifsXXuknHlUsmlgyY0PKzgPOr7/FjCePfHNt0jxm83wHZi44VDMQ7/fGNkjY3/jV1MC+1CmZbaHzugyeRtpg==", + "dev": true, + "license": "MIT" + }, "node_modules/cssstyle": { "version": "4.6.0", "resolved": "https://registry.npmjs.org/cssstyle/-/cssstyle-4.6.0.tgz", @@ -4915,6 +5078,17 @@ "node": ">=0.4.0" } }, + "node_modules/dequal": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/dequal/-/dequal-2.0.3.tgz", + "integrity": "sha512-0je+qPKHEMohvfRTCEo3CrPG6cAzAYgmzKyxRiYSSDkS6eGJdyVJm7WaYA5ECaAD9wLB2T4EEeymA5aFVcYXCA==", + "dev": true, + "license": "MIT", + "peer": true, + "engines": { + "node": ">=6" + } + }, "node_modules/detect-libc": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/detect-libc/-/detect-libc-2.1.2.tgz", @@ -4954,6 +5128,14 @@ "node": ">=0.10.0" } }, + "node_modules/dom-accessibility-api": { + "version": "0.5.16", + "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", + "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", + "dev": true, + "license": "MIT", + "peer": true + }, "node_modules/dompurify": { "version": "3.3.1", "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.3.1.tgz", @@ -6612,6 +6794,16 @@ "node": ">=0.8.19" } }, + "node_modules/indent-string": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/indent-string/-/indent-string-4.0.0.tgz", + "integrity": "sha512-EdDDZu4A2OyIK7Lr/2zG+w5jmbuk1DVBnEwREQvBzspBJkCEbRa8GxU1lghYcaGJCnRWibjDXlq779X1/y5xwg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=8" + } + }, "node_modules/inflight": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/inflight/-/inflight-1.0.6.tgz", @@ -8385,6 +8577,17 @@ "react": "^16.5.1 || ^17.0.0 || ^18.0.0 || ^19.0.0" } }, + "node_modules/lz-string": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/lz-string/-/lz-string-1.5.0.tgz", + "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", + "dev": true, + "license": "MIT", + "peer": true, + "bin": { + "lz-string": "bin/bin.js" + } + }, "node_modules/magic-string": { "version": "0.30.21", "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.30.21.tgz", @@ -8499,6 +8702,16 @@ "node": ">=6" } }, + "node_modules/min-indent": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/min-indent/-/min-indent-1.0.1.tgz", + "integrity": "sha512-I9jwMn07Sy/IwOj3zVkVik2JTvgpaykDZEigL6Rx6N9LbMywwUSMtxET+7lVoDLLd3O3IXwJwvuuns8UB/HeAg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=4" + } + }, "node_modules/minimatch": { "version": "3.1.5", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", @@ -9671,6 +9884,20 @@ "react-is": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" } }, + "node_modules/redent": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/redent/-/redent-3.0.0.tgz", + "integrity": "sha512-6tDA8g98We0zd0GvVeMT9arEOnTw9qM03L9cJXaCjrip1OO764RDBLBfrB4cwzNGDj5OA5ioymC9GkizgWJDUg==", + "dev": true, + "license": "MIT", + "dependencies": { + "indent-string": "^4.0.0", + "strip-indent": "^3.0.0" + }, + "engines": { + "node": ">=8" + } + }, "node_modules/redux": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/redux/-/redux-5.0.1.tgz", @@ -10583,6 +10810,19 @@ "node": ">=6" } }, + "node_modules/strip-indent": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/strip-indent/-/strip-indent-3.0.0.tgz", + "integrity": "sha512-laJTa3Jb+VQpaC6DseHhF7dXVqHTfJPCRDaEbid/drOhgitgYku/letMUqOXFoWV0zIIUbjpdH2t+tYj4bQMRQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "min-indent": "^1.0.0" + }, + "engines": { + "node": ">=8" + } + }, "node_modules/strip-json-comments": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-3.1.1.tgz", diff --git a/frontend/package.json b/frontend/package.json index 71a9245b4..79ddbacbb 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -39,6 +39,8 @@ "devDependencies": { "@eslint/eslintrc": "^3", "@tailwindcss/postcss": "^4", + "@testing-library/jest-dom": "^7.0.0", + "@testing-library/react": "^16.3.2", "@types/jest": "^30.0.0", "@types/node": "^20", "@types/qrcode": "^1.5.6",