diff --git a/README.md b/README.md index fa343cc3..8792f448 100644 --- a/README.md +++ b/README.md @@ -404,10 +404,13 @@ back to the addresses on record — never to the caller. deposit.timestamp)` (`InactivityWindowNotElapsed` before that). Every oracle `withdraw` sets `last_withdraw_at` and so restarts the window for all deposits in the pool; a reclaim does not. -- Returns the deposit's full original amount; partial reclaim isn't - supported, so it fails with `InsufficientBalance` if withdrawals have - left the pool below that amount. A deposit can be reclaimed only once - (`AlreadyReclaimed`). Not blocked by `pause`. +- Returns the deposit's pro-rata share of the current pool balance: + `pool.balance * deposit.amount / total_unreclaimed_deposits`. This prevents + an early claimant from taking another sponsor's share after shared + withdrawals. A reclaim can return less than the original deposit, and a + share that rounds to zero fails with `InsufficientBalance` without consuming + the claim. A deposit can be reclaimed only once (`AlreadyReclaimed`). Not + blocked by `pause`. - A pool holds at most `MAX_DEPOSITS` deposits (`TooManyDeposits` after that), which bounds the number of separately reclaimable records. @@ -774,6 +777,8 @@ release; it contains a separate generated npm package for each contract. Both scripts default to Stellar testnet but can be pointed at other networks via environment variables: +Pass signing keys through `DEPLOYER_SECRET` or `INVOKER_SECRET` environment +variables; deploy and invoke do not accept secret keys as positional arguments. ```sh RPC_URL=https://soroban-mainnet.stellar.org \ diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 22b9c1d6..ec86409d 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -2674,6 +2674,130 @@ fn test_release_rejects_self_payout() { assert_eq!(escrow.status, crate::types::EscrowStatus::Funded); } +struct EscrowSequenceRng { + state: u64, +} + +impl EscrowSequenceRng { + fn new(seed: u64) -> Self { + Self { state: seed } + } + + fn next_u32(&mut self) -> u32 { + self.state = self + .state + .wrapping_mul(6_364_136_223_846_793_005) + .wrapping_add(1_442_695_040_888_963_407); + (self.state >> 32) as u32 + } + + fn next_range(&mut self, upper_bound: u32) -> u32 { + self.next_u32() % upper_bound + } +} + +fn assert_funded_escrow_solvency( + client: &EscrowContractClient<'_>, + token_client: &token::Client<'_>, + contract_id: &Address, + first_issue_id: u64, + issue_count: u64, +) { + let mut funded_amount = 0i128; + for issue_id in first_issue_id..first_issue_id + issue_count { + let escrow = match client.try_get_escrow(&issue_id) { + Ok(Ok(escrow)) => escrow, + Err(Ok(Error::EscrowNotFound)) => continue, + result => panic!("unexpected get_escrow result for {issue_id}: {result:?}"), + }; + if escrow.status == EscrowStatus::Funded { + funded_amount = funded_amount + .checked_add(escrow.amount) + .expect("funded escrow sum should not overflow"); + } + } + + assert!( + token_client.balance(contract_id) >= funded_amount, + "contract token balance must cover the sum of all funded escrows" + ); +} + +#[test] +fn test_escrow_solvency_invariant_fuzzing() { + let env = Env::default(); + env.mock_all_auths(); + let (contract_id, _admin, _treasury, client) = setup(&env); + + let token_admin = Address::generate(&env); + let (token_addr, asset_client, token_client) = create_token(&env, &token_admin); + let sponsors = [ + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ]; + for sponsor in &sponsors { + asset_client.mint(sponsor, &10_000_000i128); + } + let recipient = Address::generate(&env); + let mut rng = EscrowSequenceRng::new(0x5eed); + let first_issue_id = 900u64; + let issue_count = 8u64; + let mut exercised_actions = [false; 4]; + + for _ in 0..500 { + let issue_id = first_issue_id + rng.next_range(issue_count as u32) as u64; + let sponsor = &sponsors[rng.next_range(sponsors.len() as u32) as usize]; + match rng.next_range(4) { + 0 => { + let amount = rng.next_range(5_000) as i128 + 1; + let deadline = env.ledger().timestamp() + 1_000; + if matches!( + client.try_fund(&issue_id, sponsor, &token_addr, &amount, &deadline, &None), + Ok(Ok(())) + ) { + exercised_actions[0] = true; + } + } + 1 => { + let amount = rng.next_range(1_000) as i128 + 1; + if matches!( + client.try_contribute(&issue_id, sponsor, &amount), + Ok(Ok(())) + ) { + exercised_actions[1] = true; + } + } + 2 => { + let recipients = vec![&env, (recipient.clone(), 10_000u32)]; + if matches!(client.try_release(&issue_id, &recipients), Ok(Ok(()))) { + exercised_actions[2] = true; + } + } + 3 => { + if matches!(client.try_refund(&issue_id), Ok(Ok(()))) { + exercised_actions[3] = true; + } + } + _ => unreachable!(), + } + + assert_funded_escrow_solvency( + &client, + &token_client, + &contract_id, + first_issue_id, + issue_count, + ); + } + + assert!( + exercised_actions.into_iter().all(|exercised| exercised), + "the randomized sequence must successfully exercise fund, contribute, release, and refund" + ); +} + // ── upgrade tests (#461) ────────────────────────────────────────────────────── #[test] diff --git a/contracts/maintenance-pool/src/lib.rs b/contracts/maintenance-pool/src/lib.rs index 55ec3cfc..a8de608f 100644 --- a/contracts/maintenance-pool/src/lib.rs +++ b/contracts/maintenance-pool/src/lib.rs @@ -432,7 +432,7 @@ mod contract { /// - The deposit index is out of range /// - The caller is not the deposit's original sponsor /// - The inactivity window hasn't elapsed since the last withdrawal - /// - The pool balance is insufficient (partial reclaim not supported) + /// - The pro-rata share of the current pool balance rounds to zero pub fn reclaim_deposit( env: Env, pool_id: u64, @@ -476,11 +476,26 @@ mod contract { return Err(Error::AlreadyReclaimed); } - if deposit.amount > pool.balance { - return Err(Error::InsufficientBalance); + let mut total_unreclaimed = 0i128; + for index in 0..pool.deposit_count { + let outstanding: Deposit = env + .storage() + .persistent() + .get(&DataKey::Deposit(pool_id, index)) + .ok_or(Error::DepositNotFound)?; + total_unreclaimed = total_unreclaimed + .checked_add(outstanding.amount) + .ok_or(Error::ArithmeticOverflow)?; } - let transfer_amount = deposit.amount; + let transfer_amount = pool + .balance + .checked_mul(deposit.amount) + .ok_or(Error::ArithmeticOverflow)? + / total_unreclaimed; + if transfer_amount == 0 { + return Err(Error::InsufficientBalance); + } // Zero the deposit record **before** the transfer so a re-entrant or // repeated call cannot drain other sponsors' funds (#479). @@ -502,7 +517,7 @@ mod contract { // pool, breaking balance invariants. (#485) pool.total_withdrawn = pool .total_withdrawn - .checked_add(deposit.amount) + .checked_add(transfer_amount) .ok_or(Error::ArithmeticOverflow)?; env.storage().persistent().set(&pkey, &pool); extend_ttl(&env, &pkey); diff --git a/contracts/maintenance-pool/src/test.rs b/contracts/maintenance-pool/src/test.rs index 6eb8b99b..55486df4 100644 --- a/contracts/maintenance-pool/src/test.rs +++ b/contracts/maintenance-pool/src/test.rs @@ -122,7 +122,7 @@ fn test_pool_lifecycle_and_admin_events() { pool_id: 445, sponsor: sponsor_a.clone(), deposit_index: 0, - amount: 1_000, + amount: 500, }, ); token_client.transfer(&sponsor_a, &contract_id, &17i128); @@ -906,6 +906,40 @@ fn test_reclaim_deposit_rejects_one_second_before_inactivity_window_boundary() { assert_eq!(err, Err(Ok(Error::InactivityWindowNotElapsed))); } +#[test] +fn test_reclaim_deposit_refunds_pro_rata_after_shared_withdrawals() { + let env = Env::default(); + env.mock_all_auths(); + let (_admin, _treasury, client) = setup(&env); + + let token_admin = Address::generate(&env); + let (token_addr, asset_client, token_client) = create_token(&env, &token_admin); + let sponsor_a = Address::generate(&env); + let sponsor_b = Address::generate(&env); + asset_client.mint(&sponsor_a, &100_0000000i128); + asset_client.mint(&sponsor_b, &100_0000000i128); + + client.deposit(&93u64, &sponsor_a, &token_addr, &100_0000000i128); + client.deposit(&93u64, &sponsor_b, &token_addr, &100_0000000i128); + + let maintainer = Address::generate(&env); + client.withdraw( + &93u64, + &vec![&env, (maintainer, 10_000u32)], + &150_0000000i128, + ); + assert_eq!(client.get_pool(&93u64).balance, 50_0000000i128); + + env.ledger().set_timestamp(INACTIVITY_WINDOW + 1); + client.reclaim_deposit(&93u64, &0u32, &sponsor_a); + assert_eq!(token_client.balance(&sponsor_a), 25_0000000i128); + assert_eq!(client.get_pool(&93u64).balance, 25_0000000i128); + + client.reclaim_deposit(&93u64, &1u32, &sponsor_b); + assert_eq!(token_client.balance(&sponsor_b), 25_0000000i128); + assert_eq!(client.get_pool(&93u64).balance, 0); +} + #[test] fn test_unpause_restores_deposit() { let env = Env::default(); diff --git a/scripts/deploy.mjs b/scripts/deploy.mjs index 36cf390a..a510f7fc 100644 --- a/scripts/deploy.mjs +++ b/scripts/deploy.mjs @@ -18,13 +18,12 @@ const RPC_URL = process.env.RPC_URL || "https://soroban-testnet.stellar.org"; const NETWORK_PASSPHRASE = process.env.NETWORK_PASSPHRASE || Networks.TESTNET; const server = new rpc.Server(RPC_URL); -const deployerSecret = process.env.DEPLOYER_SECRET || process.argv[2]; -const wasmPath = process.env.DEPLOYER_SECRET ? process.argv[2] : process.argv[3]; -const contractName = (process.env.DEPLOYER_SECRET ? process.argv[3] : process.argv[4]) ?? "contract"; +const deployerSecret = process.env.DEPLOYER_SECRET; +const wasmPath = process.argv[2]; +const contractName = process.argv[3] ?? "contract"; if (!deployerSecret || !wasmPath) { console.error("Usage: DEPLOYER_SECRET=S... node deploy.mjs [name]"); - console.error(" or: node deploy.mjs [name] (insecure fallback)"); process.exit(1); } diff --git a/scripts/deploy.test.mjs b/scripts/deploy.test.mjs index 8d339a8d..d702bb04 100644 --- a/scripts/deploy.test.mjs +++ b/scripts/deploy.test.mjs @@ -18,15 +18,16 @@ describe("scripts/deploy.mjs CLI argument validation", () => { } }); - test("fails with exit code 1 when only secret is provided without wasm path", async () => { + test("rejects a positional secret instead of treating it as credentials", async () => { try { - await execFileAsync("node", ["scripts/deploy.mjs", "SDUMMYSECRET"], { + await execFileAsync("node", ["scripts/deploy.mjs", "SDUMMYSECRET", "fake.wasm"], { env: { ...process.env, DEPLOYER_SECRET: "" }, }); assert.fail("Expected process to exit with non-zero code"); } catch (err) { assert.strictEqual(err.code, 1); - assert.match(err.stderr, /Usage/); + assert.match(err.stderr, /Usage: DEPLOYER_SECRET=S/); + assert.doesNotMatch(err.stderr, /insecure fallback/); } }); }); diff --git a/scripts/examples/deposit-withdraw-flow.sh b/scripts/examples/deposit-withdraw-flow.sh index 87064067..3a9e46ef 100755 --- a/scripts/examples/deposit-withdraw-flow.sh +++ b/scripts/examples/deposit-withdraw-flow.sh @@ -41,7 +41,7 @@ WITHDRAW_AMOUNT="${WITHDRAW_AMOUNT:-500000}" FEE_BPS="${FEE_BPS:-250}" admin_pub() { - node -e "const {Keypair}=require('@stellar/stellar-sdk'); console.log(Keypair.fromSecret(process.argv[1]).publicKey())" "$1" + ADMIN_SECRET="$1" node -e "const {Keypair}=require('@stellar/stellar-sdk'); console.log(Keypair.fromSecret(process.env.ADMIN_SECRET).publicKey())" } ADMIN_ADDRESS="$(admin_pub "$ADMIN_SECRET")" @@ -51,21 +51,21 @@ SPONSOR_ADDRESS="$(admin_pub "$SPONSOR_SECRET")" if [ -z "${CONTRACT_ID:-}" ]; then echo "==> No CONTRACT_ID given, deploying a fresh mergefi-maintenance-pool instance" - CONTRACT_ID="$(node scripts/deploy.mjs "$ADMIN_SECRET" "$WASM_PATH" maintenance-pool | tee /dev/stderr | grep -oE '[A-Z0-9]{56}')" + CONTRACT_ID="$(DEPLOYER_SECRET="$ADMIN_SECRET" node scripts/deploy.mjs "$WASM_PATH" maintenance-pool | tee /dev/stderr | grep -oE '[A-Z0-9]{56}')" fi echo "==> Using contract: $CONTRACT_ID" echo "==> 1/3 initialize(admin=$ADMIN_ADDRESS, oracle=$ORACLE_ADDRESS, treasury=$TREASURY_ADDRESS, fee_bps=$FEE_BPS)" -node scripts/invoke.mjs "$ADMIN_SECRET" "$CONTRACT_ID" initialize \ +INVOKER_SECRET="$ADMIN_SECRET" node scripts/invoke.mjs "$CONTRACT_ID" initialize \ "address:$ADMIN_ADDRESS" "address:$ORACLE_ADDRESS" "address:$TREASURY_ADDRESS" "u32:$FEE_BPS" "none" echo "==> 2/3 deposit(pool_id=$POOL_ID, sponsor=$SPONSOR_ADDRESS, token=$TOKEN, amount=$DEPOSIT_AMOUNT)" -node scripts/invoke.mjs "$SPONSOR_SECRET" "$CONTRACT_ID" deposit \ +INVOKER_SECRET="$SPONSOR_SECRET" node scripts/invoke.mjs "$CONTRACT_ID" deposit \ "u64:$POOL_ID" "address:$SPONSOR_ADDRESS" "address:$TOKEN" "i128:$DEPOSIT_AMOUNT" echo "==> 3/3 withdraw(pool_id=$POOL_ID, recipient=$ADMIN_ADDRESS, amount=$WITHDRAW_AMOUNT)" -node scripts/invoke.mjs "$ORACLE_SECRET" "$CONTRACT_ID" withdraw \ +INVOKER_SECRET="$ORACLE_SECRET" node scripts/invoke.mjs "$CONTRACT_ID" withdraw \ "u64:$POOL_ID" "address:$ADMIN_ADDRESS" "i128:$WITHDRAW_AMOUNT" echo "==> Done. Query the pool's remaining balance with:" -echo " node scripts/invoke.mjs $ADMIN_SECRET $CONTRACT_ID get_pool u64:$POOL_ID" +echo " INVOKER_SECRET=\$ADMIN_SECRET node scripts/invoke.mjs $CONTRACT_ID get_pool u64:$POOL_ID" diff --git a/scripts/invoke.mjs b/scripts/invoke.mjs index 042b2836..e318ddd3 100644 --- a/scripts/invoke.mjs +++ b/scripts/invoke.mjs @@ -1,3 +1,5 @@ +import path from "node:path"; +import { fileURLToPath } from "node:url"; import { Keypair, TransactionBuilder, @@ -16,21 +18,6 @@ const RPC_URL = process.env.RPC_URL || "https://soroban-testnet.stellar.org"; const NETWORK_PASSPHRASE = process.env.NETWORK_PASSPHRASE || Networks.TESTNET; const server = new rpc.Server(RPC_URL); -let secret = process.env.INVOKER_SECRET || process.env.DEPLOYER_SECRET; -let contractId, method, args; - -if (secret) { - [, , contractId, method, ...args] = process.argv; -} else { - [, , secret, contractId, method, ...args] = process.argv; -} - -if (!secret || !contractId || !method) { - console.error("Usage: INVOKER_SECRET=S... node invoke.mjs [args...]"); - console.error(" or: node invoke.mjs [args...] (insecure fallback)"); - process.exit(1); -} - export function parseArg(raw) { if (raw === "none") return nativeToScVal(null, { type: "void" }); @@ -78,11 +65,20 @@ export function parseArg(raw) { throw new Error(`Unknown arg type: ${type}`); } -const kp = Keypair.fromSecret(secret); -const contract = new Contract(contractId); -const scArgs = args.map(parseArg); - async function main() { + const secret = process.env.INVOKER_SECRET || process.env.DEPLOYER_SECRET; + const [, , contractId, method, ...args] = process.argv; + + if (!secret || !contractId || !method) { + console.error("Usage: INVOKER_SECRET=S... node invoke.mjs [args...]"); + console.error(" or: DEPLOYER_SECRET=S... node invoke.mjs [args...]"); + process.exitCode = 1; + return; + } + + const kp = Keypair.fromSecret(secret); + const contract = new Contract(contractId); + const scArgs = args.map(parseArg); const account = await server.getAccount(kp.publicKey()); const tx = new TransactionBuilder(account, { fee: BASE_FEE, @@ -101,7 +97,9 @@ async function main() { // to the contract that was actually called. const CONTRACT_NAME = process.env.CONTRACT_NAME ?? null; -main().catch((err) => { - console.error(formatError(err, CONTRACT_NAME)); - process.exit(1); -}); +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + main().catch((err) => { + console.error(formatError(err, CONTRACT_NAME)); + process.exit(1); + }); +} diff --git a/scripts/invoke.test.mjs b/scripts/invoke.test.mjs index 3ccb6c14..3924a27b 100644 --- a/scripts/invoke.test.mjs +++ b/scripts/invoke.test.mjs @@ -20,9 +20,26 @@ describe("scripts/invoke.mjs CLI argument validation", () => { } }); - test("fails with exit code 1 when insufficient positional arguments provided", async () => { + test("rejects a positional secret even when contract and method are provided", async () => { try { - await execFileAsync("node", ["scripts/invoke.mjs", "SDUMMYSECRET", "C123"], { + await execFileAsync( + "node", + ["scripts/invoke.mjs", "SDUMMYSECRET", "C123", "some_method"], + { + env: { ...process.env, INVOKER_SECRET: "", DEPLOYER_SECRET: "" }, + }, + ); + assert.fail("Expected process to exit with non-zero code"); + } catch (err) { + assert.strictEqual(err.code, 1); + assert.match(err.stderr, /Usage: INVOKER_SECRET=S/); + assert.doesNotMatch(err.stderr, /insecure fallback/); + } + }); + + test("fails without an env secret even when contract arguments are present", async () => { + try { + await execFileAsync("node", ["scripts/invoke.mjs", "C123"], { env: { ...process.env, INVOKER_SECRET: "", DEPLOYER_SECRET: "" }, }); assert.fail("Expected process to exit with non-zero code");