test: cover verify_prices, check contract artifacts against sources, keep secrets out of release.yml (V2-1750)

.github/scripts/check_contract_artifacts.py

@@ -0,0 +1,77 @@
+#!/usr/bin/env python3
+"""Fail if the committed PaymentVaultV2 artifacts have drifted from contracts/.
+
+Usage: check_contract_artifacts.py <forge-out-dir>
+
+Run `forge build --out <forge-out-dir>` first (contracts.yml does). The
+committed files feed `sol!` and the Anvil deploys in the tests, so if a
+contract source changes without them being regenerated, the tests keep
+exercising the old contract. Checked:
+
+  artifacts/PaymentVaultV2.json  ABI, bytecode and deployedBytecode
+  abi/IPaymentVault.json         ABI (it is PaymentVaultV2's ABI)
+
+Bytecode is compared in full, including solc's trailing CBOR metadata. That
+metadata hashes the sources and the compiler settings, so it also catches a
+drift the code alone wouldn't show. One of those settings is the remappings
+Foundry auto-detects from lib/, including OpenZeppelin's nested submodules,
+so the build only reproduces the committed artifact byte for byte when the
+submodules are checked out recursively (contracts.yml does). Nothing is
+stripped before comparing, so no executable code can be mistaken for metadata.
+
+artifacts/AutonomiNetworkToken.json is not checked: it is a Hardhat artifact
+whose source (contracts/AutonomiNetworkToken.sol) is not in this repo.
+"""
+
+import json
+import sys
+from pathlib import Path
+
+COMMITTED = Path("artifacts/PaymentVaultV2.json")
+COMMITTED_ABI = Path("abi/IPaymentVault.json")
+
+
+def canonical(value):
+    return json.dumps(value, sort_keys=True)
+
+
+def code(entry):
+    """Lower-case hex code without 0x, from a Foundry bytecode entry or a hex string."""
+    hex_code = entry["object"] if isinstance(entry, dict) else entry
+    return hex_code.removeprefix("0x").lower()
+
+
+def main():
+    if len(sys.argv) != 2:
+        sys.exit(__doc__)
+    built_path = Path(sys.argv[1]) / "PaymentVaultV2.sol" / "PaymentVaultV2.json"
+    built = json.loads(built_path.read_text())
+    committed = json.loads(COMMITTED.read_text())
+    committed_abi = json.loads(COMMITTED_ABI.read_text())
+
+    failures = []
+    if canonical(built["abi"]) != canonical(committed["abi"]):
+        failures.append(f"{COMMITTED}: ABI differs from contracts/")
+    if canonical(built["abi"]) != canonical(committed_abi):
+        failures.append(f"{COMMITTED_ABI}: ABI differs from contracts/")
+    for key in ("bytecode", "deployedBytecode"):
+        if code(built[key]) != code(committed[key]):
+            failures.append(f"{COMMITTED}: {key} differs from contracts/")
+
+    if failures:
+        for failure in failures:
+            print(f"::error::{failure}")
+        print(
+            "Regenerate with `forge build`, copy "
+            "artifacts/PaymentVaultV2.sol/PaymentVaultV2.json over "
+            f"{COMMITTED}, and write its `abi` array to {COMMITTED_ABI}.\n"
+            "If only the end of the bytecode (the metadata hash) differs, the "
+            "build probably saw different remappings: check out the submodules "
+            "recursively (`git submodule update --init --recursive`) and rebuild."
+        )
+        sys.exit(1)
+    print(f"{COMMITTED} and {COMMITTED_ABI} match contracts/")
+
+
+if __name__ == "__main__":
+    main()

.github/workflows/contracts.yml

@@ -0,0 +1,45 @@
+name: Contract artifacts
+
+# The committed PaymentVaultV2 artifacts are what the tests deploy, so check
+# they still match contracts/. Only runs when a PR touches the contracts, the
+# artifacts or the Foundry config.
+on:
+  pull_request:
+    paths:
+      - "contracts/**"
+      - "artifacts/**"
+      - "abi/**"
+      - "lib/**"
+      - "foundry.toml"
+      - "remappings.txt"
+      - ".github/scripts/check_contract_artifacts.py"
+      - ".github/workflows/contracts.yml"
+
+permissions:
+  contents: read
+
+jobs:
+  artifacts:
+    name: Artifacts match sources
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Recursive: Foundry auto-detects remappings from OpenZeppelin's nested
+          # submodules too, and they are hashed into the bytecode's metadata,
+          # which the check compares in full.
+          submodules: recursive
+
+      - name: Install Foundry
+        uses: foundry-rs/foundry-toolchain@v1
+        with:
+          # Pinned so a Foundry release can't change the build (e.g. the
+          # remappings it auto-detects) under this byte-for-byte check.
+          # Bump deliberately, with the check passing on the new version.
+          version: v1.8.7
+
+      - name: Build contracts
+        run: forge build --out "$RUNNER_TEMP/forge-out" --cache-path "$RUNNER_TEMP/forge-cache"
+
+      - name: Compare with committed artifacts
+        run: python3 .github/scripts/check_contract_artifacts.py "$RUNNER_TEMP/forge-out"

.github/workflows/release.yml

@@ -52,8 +52,11 @@ jobs:
       - shell: bash
         run: cargo binstall --no-confirm release-plz
 
+      # Secrets go through env, not ${{ }} in the script. cargo reads
+      # CARGO_REGISTRY_TOKEN when release-plz publishes, so no `cargo login`.
       - name: Publish crates
         shell: bash
-        run: |
-          cargo login "${{ secrets.CRATES_IO_TOKEN }}"
-          release-plz release --git-token ${{ secrets.GITHUB_TOKEN }}
+        env:
+          CARGO_REGISTRY_TOKEN: ${{ secrets.CRATES_IO_TOKEN }}
+          GIT_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: release-plz release --git-token "$GIT_TOKEN"

foundry.toml

@@ -5,5 +5,10 @@ libs = ["lib"]
 optimizer = true
 optimizer_runs = 200
 via_ir = true
+# The committed artifacts/PaymentVaultV2.json was built with these. Pinned so a
+# rebuild (and the contracts.yml drift check) reproduces its bytecode instead of
+# picking up whatever solc / EVM version Foundry defaults to that day.
+solc_version = "0.8.28"
+evm_version = "cancun"
 
 # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

src/merkle_payments/merkle_payment.rs

@@ -276,3 +276,112 @@ impl MerklePaymentProof {
         self.winner_pool.hash()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::merkle_payments::MerkleTree;
+
+    const TIMESTAMP: u64 = 1_700_000_000;
+
+    /// A pool whose candidate `i` quotes `100 + i`, so every index has a
+    /// distinct price and a mismatch report names the right candidate.
+    fn make_pool() -> MerklePaymentCandidatePool {
+        let leaves = (0..4u8).map(|i| XorName([i; 32])).collect();
+        let tree = MerkleTree::from_xornames(leaves).expect("tree");
+        let midpoint_proof = tree
+            .reward_candidates(TIMESTAMP)
+            .expect("reward candidates")
+            .remove(0);
+        let candidate_nodes = std::array::from_fn(|i| MerklePaymentCandidateNode {
+            pub_key: vec![i as u8; 4],
+            price: Amount::from(100 + i as u64),
+            reward_address: RewardsAddress::new([i as u8; 20]),
+            merkle_payment_timestamp: TIMESTAMP,
+            signature: vec![],
+            committed_key_count: 0,
+            commitment_pin: None,
+        });
+        MerklePaymentCandidatePool {
+            midpoint_proof,
+            candidate_nodes,
+        }
+    }
+
+    #[test]
+    fn verify_prices_accepts_matching_on_chain_prices() {
+        let pool = make_pool();
+        let on_chain = [pool.to_commitment()];
+        assert_eq!(pool.verify_prices(&on_chain, &pool.hash()), Ok(()));
+    }
+
+    #[test]
+    fn verify_prices_checks_the_winner_not_the_first_commitment() {
+        let pool = make_pool();
+        // A decoy pool listed first, with a different hash and every price off.
+        let mut decoy = pool.to_commitment();
+        decoy.pool_hash = [0xAA; 32];
+        for candidate in &mut decoy.candidates {
+            candidate.price += Amount::from(1u64);
+        }
+        let on_chain = [decoy, pool.to_commitment()];
+        assert_eq!(pool.verify_prices(&on_chain, &pool.hash()), Ok(()));
+    }
+
+    #[test]
+    fn verify_prices_rejects_a_mismatch_at_the_first_candidate() {
+        let pool = make_pool();
+        let mut commitment = pool.to_commitment();
+        commitment.candidates[0].price = Amount::from(999u64);
+        assert_eq!(
+            pool.verify_prices(&[commitment], &pool.hash()),
+            Err(MerklePaymentVerificationError::PriceMismatch {
+                index: 0,
+                on_chain_price: "999".to_string(),
+                expected_price: "100".to_string(),
+            })
+        );
+    }
+
+    #[test]
+    fn verify_prices_rejects_a_mismatch_at_the_last_candidate() {
+        let pool = make_pool();
+        let last = CANDIDATES_PER_POOL - 1;
+        let mut commitment = pool.to_commitment();
+        commitment.candidates[last].price = Amount::from(999u64);
+        assert_eq!(
+            pool.verify_prices(&[commitment], &pool.hash()),
+            Err(MerklePaymentVerificationError::PriceMismatch {
+                index: last,
+                on_chain_price: "999".to_string(),
+                expected_price: (100 + last).to_string(),
+            })
+        );
+    }
+
+    #[test]
+    fn verify_prices_reports_the_first_of_several_mismatches() {
+        let pool = make_pool();
+        let mut commitment = pool.to_commitment();
+        commitment.candidates[3].price = Amount::ZERO;
+        commitment.candidates[7].price = Amount::ZERO;
+        assert!(matches!(
+            pool.verify_prices(&[commitment], &pool.hash()),
+            Err(MerklePaymentVerificationError::PriceMismatch { index: 3, .. })
+        ));
+    }
+
+    #[test]
+    fn verify_prices_rejects_a_winner_hash_missing_from_the_commitments() {
+        let pool = make_pool();
+        let on_chain = [pool.to_commitment()];
+        assert_eq!(
+            pool.verify_prices(&on_chain, &[0xBB; 32]),
+            Err(MerklePaymentVerificationError::WinnerPoolNotInCommitments)
+        );
+        assert_eq!(
+            pool.verify_prices(&[], &pool.hash()),
+            Err(MerklePaymentVerificationError::WinnerPoolNotInCommitments)
+        );
+    }
+}