From d5fe55a78b94749526641cd7d10237029d25556f Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Fri, 30 Oct 2026 10:35:14 +0000 Subject: [PATCH 1/6] test: cover verify_prices, check contract artifacts against sources, keep secrets out of release.yml scripts Follow-ups from the faux-CI audit (V2-1750, part of V2-1746). - Unit tests for MerklePaymentCandidatePool::verify_prices: matching prices pass; the winner is looked up by hash, not position; mismatches at candidate 0 and 15 are rejected with the right index; the first of several mismatches is reported; a winner hash missing from the commitments is rejected. - release.yml: pass CRATES_IO_TOKEN and GITHUB_TOKEN through step env (CARGO_REGISTRY_TOKEN, GIT_TOKEN) instead of interpolating them into the script; drop `cargo login`. - foundry.toml: pin solc 0.8.28 and EVM cancun, the settings recorded in the committed PaymentVaultV2 artifact. - contracts.yml (path-filtered) + .github/scripts/check_contract_artifacts.py: rebuild the contracts and fail if artifacts/PaymentVaultV2.json or abi/IPaymentVault.json no longer match them (ABI exactly, bytecode without the CBOR metadata tail). Closes V2-1750 Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_contract_artifacts.py | 74 +++++++++++++ .github/workflows/contracts.yml | 37 +++++++ .github/workflows/release.yml | 9 +- foundry.toml | 5 + src/merkle_payments/merkle_payment.rs | 109 ++++++++++++++++++++ 5 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 .github/scripts/check_contract_artifacts.py create mode 100644 .github/workflows/contracts.yml diff --git a/.github/scripts/check_contract_artifacts.py b/.github/scripts/check_contract_artifacts.py new file mode 100644 index 0000000..77c555a --- /dev/null +++ b/.github/scripts/check_contract_artifacts.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +"""Fail if the committed PaymentVaultV2 artifacts have drifted from contracts/. + +Usage: check_contract_artifacts.py + +Run `forge build --out ` 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 without the trailing CBOR metadata. That blob hashes the +compiler settings, including the remappings Foundry auto-detects from whatever +nested submodules happen to be checked out, so it differs between otherwise +identical builds. The code before it is deterministic for a pinned solc. + +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): + """Hex code without 0x and without the CBOR metadata tail.""" + hex_code = entry["object"] if isinstance(entry, dict) else entry + hex_code = hex_code.removeprefix("0x") + # The last 2 bytes give the length of the CBOR blob that precedes them. + metadata_len = int(hex_code[-4:], 16) * 2 + 4 + return hex_code[:-metadata_len] + + +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}." + ) + sys.exit(1) + print(f"{COMMITTED} and {COMMITTED_ABI} match contracts/") + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml new file mode 100644 index 0000000..7b6fd4f --- /dev/null +++ b/.github/workflows/contracts.yml @@ -0,0 +1,37 @@ +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: + submodules: true + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - 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" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 96f90f2..cc1a7b6 100644 --- a/.github/workflows/release.yml +++ b/.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" diff --git a/foundry.toml b/foundry.toml index 720bd2c..760a06a 100644 --- a/foundry.toml +++ b/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 diff --git a/src/merkle_payments/merkle_payment.rs b/src/merkle_payments/merkle_payment.rs index 9bd6ed0..09cb859 100644 --- a/src/merkle_payments/merkle_payment.rs +++ b/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) + ); + } +} From e1d4c44eda5284560f9f183338cac738a56c186a Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Mon, 2 Nov 2026 10:00:39 +0000 Subject: [PATCH 2/6] fix: only strip a genuine solc metadata tail when comparing bytecode (V2-1750) Hermes round 1 (REQUEST_CHANGES on d5fe55a7): code() trusted the last two bytes as a CBOR length. With no metadata (e.g. cbor_metadata = false in foundry.toml, which triggers this workflow) the "length" is just code, and an oversized one stripped all executable code: code("0x600160005500") == code("0x600260005500") == "". The drift check could then pass despite a code change. metadata_length() now accepts the tail only if the bytes before the footer are exactly one well-formed CBOR map of the stated length whose keys are all solc metadata keys (ipfs, bzzr0, bzzr1, solc, experimental). Otherwise the full bytecode is compared. test_check_contract_artifacts.py (11 tests, run by contracts.yml before the comparison): a solc-shaped tail is stripped; builds differing only in metadata compare equal; different code with the same tail differs; metadata-free, oversized-footer, truncated, non-CBOR, short-map and unknown-key inputs are compared in full; the committed artifact's tail is recognised. Three of them fail against the old parser. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_contract_artifacts.py | 83 +++++++++++++++- .../scripts/test_check_contract_artifacts.py | 96 +++++++++++++++++++ .github/workflows/contracts.yml | 4 + 3 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 .github/scripts/test_check_contract_artifacts.py diff --git a/.github/scripts/check_contract_artifacts.py b/.github/scripts/check_contract_artifacts.py index 77c555a..a2cec67 100644 --- a/.github/scripts/check_contract_artifacts.py +++ b/.github/scripts/check_contract_artifacts.py @@ -16,6 +16,12 @@ nested submodules happen to be checked out, so it differs between otherwise identical builds. The code before it is deterministic for a pinned solc. +The tail is only stripped when it really is solc metadata: the last two bytes +give a length, and the bytes before them must be exactly one well-formed CBOR +map of that length whose keys are all known metadata keys. Anything else +(no metadata, a truncated or oversized footer, non-CBOR bytes) is compared as +full bytecode, so a check can never pass by stripping executable code. + artifacts/AutonomiNetworkToken.json is not checked: it is a Hardhat artifact whose source (contracts/AutonomiNetworkToken.sol) is not in this repo. """ @@ -32,13 +38,80 @@ def canonical(value): return json.dumps(value, sort_keys=True) +# Keys solc writes into the metadata map (bytecodeHash ipfs/bzzr0/bzzr1, the +# compiler version, and the experimental flag). +METADATA_KEYS = {"ipfs", "bzzr0", "bzzr1", "solc", "experimental"} + + +def _cbor_item(data, pos): + """Parse one CBOR item of the kinds solc metadata uses. + + Returns (value, next_pos), or None if `data` doesn't hold such an item at + `pos`. Supports unsigned ints, byte and text strings, maps, and the + simple values false/true. Strings are returned as bytes. + """ + if pos >= len(data): + return None + major, info = data[pos] >> 5, data[pos] & 0x1F + pos += 1 + if major == 7: + return (info == 21, pos) if info in (20, 21) else None + if info < 24: + arg = info + elif info in (24, 25): + width = 1 if info == 24 else 2 + if pos + width > len(data): + return None + arg = int.from_bytes(data[pos:pos + width], "big") + pos += width + else: + return None + if major == 0: + return arg, pos + if major in (2, 3): + if pos + arg > len(data): + return None + return bytes(data[pos:pos + arg]), pos + arg + if major == 5: + items = {} + for _ in range(arg): + key = _cbor_item(data, pos) + if key is None or not isinstance(key[0], bytes): + return None + value = _cbor_item(data, key[1]) + if value is None: + return None + items[key[0]] = value[0] + pos = value[1] + return items, pos + return None + + +def metadata_length(raw): + """Length in bytes of solc's metadata tail (CBOR map + 2-byte length), or 0.""" + if len(raw) < 3: + return 0 + cbor_len = int.from_bytes(raw[-2:], "big") + if cbor_len == 0 or cbor_len + 2 > len(raw): + return 0 + blob = raw[-2 - cbor_len:-2] + if blob[0] >> 5 != 5: # must be a CBOR map + return 0 + parsed = _cbor_item(blob, 0) + if parsed is None or parsed[1] != len(blob): + return 0 # malformed, or doesn't fill exactly the stated length + keys = {key.decode("utf-8", "replace") for key in parsed[0]} + if not keys or not keys <= METADATA_KEYS: + return 0 + return cbor_len + 2 + + def code(entry): - """Hex code without 0x and without the CBOR metadata tail.""" + """Hex code without 0x and without solc's metadata tail, when there is one.""" hex_code = entry["object"] if isinstance(entry, dict) else entry - hex_code = hex_code.removeprefix("0x") - # The last 2 bytes give the length of the CBOR blob that precedes them. - metadata_len = int(hex_code[-4:], 16) * 2 + 4 - return hex_code[:-metadata_len] + raw = bytes.fromhex(hex_code.removeprefix("0x")) + tail = metadata_length(raw) + return (raw[:-tail] if tail else raw).hex() def main(): diff --git a/.github/scripts/test_check_contract_artifacts.py b/.github/scripts/test_check_contract_artifacts.py new file mode 100644 index 0000000..f3970ea --- /dev/null +++ b/.github/scripts/test_check_contract_artifacts.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +"""Tests for check_contract_artifacts.py's metadata stripping. + +Run: python3 .github/scripts/test_check_contract_artifacts.py (contracts.yml +does, before the artifact comparison). +""" + +import json +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from check_contract_artifacts import code, metadata_length # noqa: E402 + +REPO = Path(__file__).resolve().parents[2] + + +def solc_tail(ipfs_byte=0x11, version=(0, 8, 28)): + """A metadata tail shaped like solc's: {ipfs: <34 bytes>, solc: <3 bytes>} + length.""" + blob = ( + bytes([0xA2]) + + bytes([0x64]) + b"ipfs" + bytes([0x58, 0x22]) + bytes([ipfs_byte]) * 34 + + bytes([0x64]) + b"solc" + bytes([0x43]) + bytes(version) + ) + return blob + len(blob).to_bytes(2, "big") + + +EXECUTABLE = bytes.fromhex("6080604052348015600e575f5ffd5b50") + + +class MetadataStripping(unittest.TestCase): + def test_strips_a_solc_shaped_tail(self): + raw = EXECUTABLE + solc_tail() + self.assertEqual(metadata_length(raw), len(solc_tail())) + self.assertEqual(code(raw.hex()), EXECUTABLE.hex()) + + def test_builds_differing_only_in_metadata_compare_equal(self): + a = "0x" + (EXECUTABLE + solc_tail(ipfs_byte=0x11)).hex() + b = "0x" + (EXECUTABLE + solc_tail(ipfs_byte=0x22)).hex() + self.assertEqual(code(a), code(b)) + + def test_different_code_with_the_same_tail_still_differs(self): + tail = solc_tail() + a = "0x" + (bytes.fromhex("600160005500") + tail).hex() + b = "0x" + (bytes.fromhex("600260005500") + tail).hex() + self.assertNotEqual(code(a), code(b)) + + def test_metadata_free_code_is_compared_in_full(self): + # Hermes's reproduction: the old parser read 0x5500 as a length and + # stripped everything, so these compared equal. + self.assertEqual(code("0x600160005500"), "600160005500") + self.assertNotEqual(code("0x600160005500"), code("0x600260005500")) + + def test_oversized_footer_is_not_stripped(self): + raw = EXECUTABLE + bytes([0xFF, 0xFF]) + self.assertEqual(metadata_length(raw), 0) + self.assertEqual(code(raw.hex()), raw.hex()) + + def test_truncated_tail_is_not_stripped(self): + # Drop bytes from the front of a valid tail: the footer now claims more + # CBOR than is left before it. + raw = solc_tail()[10:] + self.assertEqual(metadata_length(raw), 0) + + def test_footer_over_non_cbor_bytes_is_not_stripped(self): + raw = EXECUTABLE + bytes([0x60, 0x01, 0x55]) + (3).to_bytes(2, "big") + self.assertEqual(metadata_length(raw), 0) + + def test_map_shorter_than_the_footer_is_not_stripped(self): + blob = bytes([0xA1, 0x64]) + b"solc" + bytes([0x43, 0, 8, 28]) + padded = blob + bytes([0x00]) # one byte of garbage inside the stated length + raw = EXECUTABLE + padded + len(padded).to_bytes(2, "big") + self.assertEqual(metadata_length(raw), 0) + + def test_unknown_keys_are_not_metadata(self): + blob = bytes([0xA1, 0x63]) + b"foo" + bytes([0x01]) + raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") + self.assertEqual(metadata_length(raw), 0) + + def test_empty_and_tiny_code(self): + self.assertEqual(code("0x"), "") + self.assertEqual(code("0x00"), "00") + + def test_committed_artifact_has_a_recognised_tail(self): + artifact = json.loads((REPO / "artifacts" / "PaymentVaultV2.json").read_text()) + for key in ("bytecode", "deployedBytecode"): + entry = artifact[key] + hex_code = entry["object"] if isinstance(entry, dict) else entry + raw = bytes.fromhex(hex_code.removeprefix("0x")) + self.assertGreater(metadata_length(raw), 0, key) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index 7b6fd4f..c07d234 100644 --- a/.github/workflows/contracts.yml +++ b/.github/workflows/contracts.yml @@ -13,6 +13,7 @@ on: - "foundry.toml" - "remappings.txt" - ".github/scripts/check_contract_artifacts.py" + - ".github/scripts/test_check_contract_artifacts.py" - ".github/workflows/contracts.yml" permissions: @@ -30,6 +31,9 @@ jobs: - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 + - name: Test the artifact checker + run: python3 .github/scripts/test_check_contract_artifacts.py + - name: Build contracts run: forge build --out "$RUNNER_TEMP/forge-out" --cache-path "$RUNNER_TEMP/forge-cache" From 2a2722d0d9a0ef7d1b83525f8859db7e6bcfd05f Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Mon, 2 Nov 2026 11:45:53 +0000 Subject: [PATCH 3/6] fix: strip only a tail shaped exactly like solc's metadata (V2-1750) Hermes round 2 (REQUEST_CHANGES on e1d4c44): whitelisting map keys doesn't establish solc metadata. "...a164736f6c63010007" ({"solc": 1} plus its length) was stripped although those bytes also execute (LOG1/PUSH5/STOP), so changed code could compare equal. The parser also accepted byte-string keys and silently kept the last duplicate. _is_solc_metadata now requires: unique CBOR text-string keys; `solc` present, as a 3-byte release version or a non-empty prerelease text; at most one hash, ipfs as 34 bytes or bzzr0/bzzr1 as 32 bytes; `experimental` only as true; no other keys. Anything else is compared as full bytecode. Tests (17, up from 11): the reviewer's counterexample, duplicate keys, a byte-string key, wrong value types/lengths, missing solc, two hashes, and the shapes solc does emit (ipfs + release, bytecodeHash none, prerelease, bzzr1 + experimental). 5 of the new tests fail against the round-1 parser. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_contract_artifacts.py | 73 ++++++++++++++----- .../scripts/test_check_contract_artifacts.py | 53 ++++++++++++++ 2 files changed, 106 insertions(+), 20 deletions(-) diff --git a/.github/scripts/check_contract_artifacts.py b/.github/scripts/check_contract_artifacts.py index a2cec67..66050a2 100644 --- a/.github/scripts/check_contract_artifacts.py +++ b/.github/scripts/check_contract_artifacts.py @@ -18,9 +18,11 @@ The tail is only stripped when it really is solc metadata: the last two bytes give a length, and the bytes before them must be exactly one well-formed CBOR -map of that length whose keys are all known metadata keys. Anything else -(no metadata, a truncated or oversized footer, non-CBOR bytes) is compared as -full bytecode, so a check can never pass by stripping executable code. +map of that length, shaped like solc's (unique text keys, `solc` present, and +each value of the type and length solc writes; see HASH_LENGTHS below). +Anything else (no metadata, a truncated or oversized footer, non-CBOR bytes, a +map that merely looks like metadata) is compared as full bytecode, so a check +can't pass by stripping executable code. artifacts/AutonomiNetworkToken.json is not checked: it is a Hardhat artifact whose source (contracts/AutonomiNetworkToken.sol) is not in this repo. @@ -38,24 +40,31 @@ def canonical(value): return json.dumps(value, sort_keys=True) -# Keys solc writes into the metadata map (bytecodeHash ipfs/bzzr0/bzzr1, the -# compiler version, and the experimental flag). -METADATA_KEYS = {"ipfs", "bzzr0", "bzzr1", "solc", "experimental"} +# What solc writes into the metadata map. Each key appears at most once, as a +# CBOR text string, with exactly this value shape: +# ipfs byte string, 34 bytes (sha2-256 multihash) +# bzzr0, bzzr1 byte string, 32 bytes +# solc byte string, 3 bytes (release x.y.z), or a text string +# (prerelease version) +# experimental true +# `solc` is required: every build that appends a metadata map includes it. +HASH_LENGTHS = {"ipfs": 34, "bzzr0": 32, "bzzr1": 32} def _cbor_item(data, pos): """Parse one CBOR item of the kinds solc metadata uses. - Returns (value, next_pos), or None if `data` doesn't hold such an item at - `pos`. Supports unsigned ints, byte and text strings, maps, and the - simple values false/true. Strings are returned as bytes. + Returns ((kind, value), next_pos), or None if `data` doesn't hold such an + item at `pos`. Kinds: "uint", "bytes", "text", "bool", and "map", whose + value is a list of (key, value) pairs in order (duplicates kept, so the + caller can reject them). """ if pos >= len(data): return None major, info = data[pos] >> 5, data[pos] & 0x1F pos += 1 if major == 7: - return (info == 21, pos) if info in (20, 21) else None + return (("bool", info == 21), pos) if info in (20, 21) else None if info < 24: arg = info elif info in (24, 25): @@ -67,26 +76,52 @@ def _cbor_item(data, pos): else: return None if major == 0: - return arg, pos + return ("uint", arg), pos if major in (2, 3): if pos + arg > len(data): return None - return bytes(data[pos:pos + arg]), pos + arg + raw = bytes(data[pos:pos + arg]) + if major == 2: + return ("bytes", raw), pos + arg + try: + return ("text", raw.decode("utf-8")), pos + arg + except UnicodeDecodeError: + return None if major == 5: - items = {} + pairs = [] for _ in range(arg): key = _cbor_item(data, pos) - if key is None or not isinstance(key[0], bytes): + if key is None: return None value = _cbor_item(data, key[1]) if value is None: return None - items[key[0]] = value[0] + pairs.append((key[0], value[0])) pos = value[1] - return items, pos + return ("map", pairs), pos return None +def _is_solc_metadata(pairs): + """True only for a map shaped exactly like solc's metadata (see above).""" + seen = set() + for (key_kind, key), (kind, value) in pairs: + if key_kind != "text" or key in seen: + return False + seen.add(key) + if key in HASH_LENGTHS: + ok = kind == "bytes" and len(value) == HASH_LENGTHS[key] + elif key == "solc": + ok = (kind == "bytes" and len(value) == 3) or (kind == "text" and value != "") + elif key == "experimental": + ok = kind == "bool" and value + else: + ok = False + if not ok: + return False + return "solc" in seen and len(seen & HASH_LENGTHS.keys()) <= 1 + + def metadata_length(raw): """Length in bytes of solc's metadata tail (CBOR map + 2-byte length), or 0.""" if len(raw) < 3: @@ -95,13 +130,11 @@ def metadata_length(raw): if cbor_len == 0 or cbor_len + 2 > len(raw): return 0 blob = raw[-2 - cbor_len:-2] - if blob[0] >> 5 != 5: # must be a CBOR map - return 0 parsed = _cbor_item(blob, 0) if parsed is None or parsed[1] != len(blob): return 0 # malformed, or doesn't fill exactly the stated length - keys = {key.decode("utf-8", "replace") for key in parsed[0]} - if not keys or not keys <= METADATA_KEYS: + (kind, pairs), _ = parsed + if kind != "map" or not _is_solc_metadata(pairs): return 0 return cbor_len + 2 diff --git a/.github/scripts/test_check_contract_artifacts.py b/.github/scripts/test_check_contract_artifacts.py index f3970ea..4a66be8 100644 --- a/.github/scripts/test_check_contract_artifacts.py +++ b/.github/scripts/test_check_contract_artifacts.py @@ -79,6 +79,59 @@ def test_unknown_keys_are_not_metadata(self): raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") self.assertEqual(metadata_length(raw), 0) + # Round 2: maps that are well-formed CBOR but not solc metadata. + + def test_reviewer_counterexample_is_compared_in_full(self): + # {"solc": 1} plus its length. Valid CBOR with a whitelisted key, but solc + # writes a 3-byte version, and these bytes also execute (LOG1/PUSH5/STOP). + self.assertEqual(code("0x600060006000a164736f6c63010007"), "600060006000a164736f6c63010007") + self.assertNotEqual(code("0x600060006000a164736f6c63010007"), code("0x600060006000")) + + def assert_not_metadata(self, blob, why): + raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") + self.assertEqual(metadata_length(raw), 0, why) + + def assert_metadata(self, blob, why): + raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") + self.assertEqual(metadata_length(raw), len(blob) + 2, why) + self.assertEqual(code(raw.hex()), EXECUTABLE.hex(), why) + + SOLC = bytes([0x64]) + b"solc" + bytes([0x43, 0, 8, 28]) + IPFS = bytes([0x64]) + b"ipfs" + bytes([0x58, 0x22]) + bytes([0x11]) * 34 + + def test_duplicate_keys_are_not_metadata(self): + self.assert_not_metadata(bytes([0xA2]) + self.SOLC + self.SOLC, "duplicate solc") + + def test_byte_string_keys_are_not_metadata(self): + key_as_bytes = bytes([0x44]) + b"solc" + bytes([0x43, 0, 8, 28]) + self.assert_not_metadata(bytes([0xA1]) + key_as_bytes, "byte-string key") + + def test_wrong_value_types_and_lengths_are_not_metadata(self): + cases = { + "solc as uint": bytes([0xA1, 0x64]) + b"solc" + bytes([0x01]), + "solc 2 bytes": bytes([0xA1, 0x64]) + b"solc" + bytes([0x42, 0, 8]), + "solc empty text": bytes([0xA1, 0x64]) + b"solc" + bytes([0x60]), + "ipfs 33 bytes": bytes([0xA2, 0x64]) + b"ipfs" + bytes([0x58, 0x21]) + bytes(33) + self.SOLC, + "ipfs as text": bytes([0xA2, 0x64]) + b"ipfs" + bytes([0x63]) + b"abc" + self.SOLC, + "experimental false": bytes([0xA2, 0x6C]) + b"experimental" + bytes([0xF4]) + self.SOLC, + } + for why, blob in cases.items(): + self.assert_not_metadata(blob, why) + + def test_solc_is_required_and_one_hash_at_most(self): + self.assert_not_metadata(bytes([0xA1]) + self.IPFS, "no solc") + bzzr1 = bytes([0x65]) + b"bzzr1" + bytes([0x58, 0x20]) + bytes(32) + self.assert_not_metadata(bytes([0xA3]) + self.IPFS + bzzr1 + self.SOLC, "two hashes") + + def test_shapes_solc_emits_are_metadata(self): + self.assert_metadata(bytes([0xA2]) + self.IPFS + self.SOLC, "ipfs + release solc") + self.assert_metadata(bytes([0xA1]) + self.SOLC, "bytecodeHash none") + prerelease = bytes([0x64]) + b"solc" + bytes([0x78, 0x18]) + b"0.8.29-nightly.2025.1.1+" + self.assert_metadata(bytes([0xA2]) + self.IPFS + prerelease, "prerelease solc") + bzzr1 = bytes([0x65]) + b"bzzr1" + bytes([0x58, 0x20]) + bytes(32) + experimental = bytes([0x6C]) + b"experimental" + bytes([0xF5]) + self.assert_metadata(bytes([0xA3]) + bzzr1 + experimental + self.SOLC, "bzzr1 + experimental") + def test_empty_and_tiny_code(self): self.assertEqual(code("0x"), "") self.assertEqual(code("0x00"), "00") From 729027492acaf77429def82f43022a5f947f398c Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Mon, 2 Nov 2026 15:38:27 +0000 Subject: [PATCH 4/6] fix: strip solc metadata only when it names the pinned compiler (V2-1750) Review of 2a2722d: the checker accepted any 3-byte `solc` value, so {"solc": h'ffffff'} (LOG1/PUSH5/SELFDESTRUCT/STOP, valid code) was stripped as metadata and different bytecode could compare equal. The `solc` value must now equal foundry.toml's [profile.default] solc_version (0.8.28 -> 00 08 1c), read from foundry.toml so the two can't drift. Prerelease (text) versions are rejected too, since the pin is a release. Tests: the reviewer's counterexample, neighbouring versions, prerelease, and the version parser; the fixtures now derive the version from the pin. The new tests fail against 2a2722d's checker. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_contract_artifacts.py | 49 ++++++++++++++++--- .../scripts/test_check_contract_artifacts.py | 49 ++++++++++++++++--- 2 files changed, 82 insertions(+), 16 deletions(-) diff --git a/.github/scripts/check_contract_artifacts.py b/.github/scripts/check_contract_artifacts.py index 66050a2..88ff7b7 100644 --- a/.github/scripts/check_contract_artifacts.py +++ b/.github/scripts/check_contract_artifacts.py @@ -18,22 +18,54 @@ The tail is only stripped when it really is solc metadata: the last two bytes give a length, and the bytes before them must be exactly one well-formed CBOR -map of that length, shaped like solc's (unique text keys, `solc` present, and -each value of the type and length solc writes; see HASH_LENGTHS below). -Anything else (no metadata, a truncated or oversized footer, non-CBOR bytes, a -map that merely looks like metadata) is compared as full bytecode, so a check -can't pass by stripping executable code. +map of that length, shaped like solc's (unique text keys, each value of the +type and length solc writes; see HASH_LENGTHS below) and naming the compiler +foundry.toml pins (`solc_version`) as its `solc` version. Anything else (no +metadata, a truncated or oversized footer, non-CBOR bytes, a map that merely +looks like metadata, another compiler version) is compared as full bytecode, +so a check can't pass by stripping executable code. artifacts/AutonomiNetworkToken.json is not checked: it is a Hardhat artifact whose source (contracts/AutonomiNetworkToken.sol) is not in this repo. """ import json +import re import sys +import tomllib from pathlib import Path COMMITTED = Path("artifacts/PaymentVaultV2.json") COMMITTED_ABI = Path("abi/IPaymentVault.json") +FOUNDRY_TOML = Path(__file__).resolve().parents[2] / "foundry.toml" + + +def solc_version_bytes(version): + """The 3 bytes solc writes as `solc` in its metadata for release `x.y.z`. + + Raises ValueError for anything else (ranges, prereleases, components > 255). + """ + match = re.fullmatch(r"(\d+)\.(\d+)\.(\d+)", version) + if not match or any(int(part) > 255 for part in match.groups()): + raise ValueError(f"not a release solc version: {version!r}") + return bytes(int(part) for part in match.groups()) + + +def pinned_solc(): + """foundry.toml's [profile.default] solc_version, as solc writes it in metadata.""" + config = tomllib.loads(FOUNDRY_TOML.read_text()) + version = config.get("profile", {}).get("default", {}).get("solc_version") + if not isinstance(version, str): + sys.exit(f"{FOUNDRY_TOML}: [profile.default] solc_version is not set") + try: + return solc_version_bytes(version) + except ValueError as e: + sys.exit(f"{FOUNDRY_TOML}: {e}") + + +# The compiler the artifacts are built with. A metadata tail naming any other +# version can't come from this build, so it is not stripped. +PINNED_SOLC = pinned_solc() def canonical(value): @@ -44,8 +76,9 @@ def canonical(value): # CBOR text string, with exactly this value shape: # ipfs byte string, 34 bytes (sha2-256 multihash) # bzzr0, bzzr1 byte string, 32 bytes -# solc byte string, 3 bytes (release x.y.z), or a text string -# (prerelease version) +# solc byte string, 3 bytes (release x.y.z); must be PINNED_SOLC. +# (A prerelease solc writes a text string here instead; the +# pin is a release, so that is rejected too.) # experimental true # `solc` is required: every build that appends a metadata map includes it. HASH_LENGTHS = {"ipfs": 34, "bzzr0": 32, "bzzr1": 32} @@ -112,7 +145,7 @@ def _is_solc_metadata(pairs): if key in HASH_LENGTHS: ok = kind == "bytes" and len(value) == HASH_LENGTHS[key] elif key == "solc": - ok = (kind == "bytes" and len(value) == 3) or (kind == "text" and value != "") + ok = kind == "bytes" and value == PINNED_SOLC elif key == "experimental": ok = kind == "bool" and value else: diff --git a/.github/scripts/test_check_contract_artifacts.py b/.github/scripts/test_check_contract_artifacts.py index 4a66be8..252cecf 100644 --- a/.github/scripts/test_check_contract_artifacts.py +++ b/.github/scripts/test_check_contract_artifacts.py @@ -12,12 +12,17 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) -from check_contract_artifacts import code, metadata_length # noqa: E402 +from check_contract_artifacts import ( # noqa: E402 + PINNED_SOLC, + code, + metadata_length, + solc_version_bytes, +) REPO = Path(__file__).resolve().parents[2] -def solc_tail(ipfs_byte=0x11, version=(0, 8, 28)): +def solc_tail(ipfs_byte=0x11, version=PINNED_SOLC): """A metadata tail shaped like solc's: {ipfs: <34 bytes>, solc: <3 bytes>} + length.""" blob = ( bytes([0xA2]) @@ -69,7 +74,7 @@ def test_footer_over_non_cbor_bytes_is_not_stripped(self): self.assertEqual(metadata_length(raw), 0) def test_map_shorter_than_the_footer_is_not_stripped(self): - blob = bytes([0xA1, 0x64]) + b"solc" + bytes([0x43, 0, 8, 28]) + blob = bytes([0xA1, 0x64]) + b"solc" + bytes([0x43]) + PINNED_SOLC padded = blob + bytes([0x00]) # one byte of garbage inside the stated length raw = EXECUTABLE + padded + len(padded).to_bytes(2, "big") self.assertEqual(metadata_length(raw), 0) @@ -96,20 +101,20 @@ def assert_metadata(self, blob, why): self.assertEqual(metadata_length(raw), len(blob) + 2, why) self.assertEqual(code(raw.hex()), EXECUTABLE.hex(), why) - SOLC = bytes([0x64]) + b"solc" + bytes([0x43, 0, 8, 28]) + SOLC = bytes([0x64]) + b"solc" + bytes([0x43]) + PINNED_SOLC IPFS = bytes([0x64]) + b"ipfs" + bytes([0x58, 0x22]) + bytes([0x11]) * 34 def test_duplicate_keys_are_not_metadata(self): self.assert_not_metadata(bytes([0xA2]) + self.SOLC + self.SOLC, "duplicate solc") def test_byte_string_keys_are_not_metadata(self): - key_as_bytes = bytes([0x44]) + b"solc" + bytes([0x43, 0, 8, 28]) + key_as_bytes = bytes([0x44]) + b"solc" + bytes([0x43]) + PINNED_SOLC self.assert_not_metadata(bytes([0xA1]) + key_as_bytes, "byte-string key") def test_wrong_value_types_and_lengths_are_not_metadata(self): cases = { "solc as uint": bytes([0xA1, 0x64]) + b"solc" + bytes([0x01]), - "solc 2 bytes": bytes([0xA1, 0x64]) + b"solc" + bytes([0x42, 0, 8]), + "solc 2 bytes": bytes([0xA1, 0x64]) + b"solc" + bytes([0x42]) + PINNED_SOLC[:2], "solc empty text": bytes([0xA1, 0x64]) + b"solc" + bytes([0x60]), "ipfs 33 bytes": bytes([0xA2, 0x64]) + b"ipfs" + bytes([0x58, 0x21]) + bytes(33) + self.SOLC, "ipfs as text": bytes([0xA2, 0x64]) + b"ipfs" + bytes([0x63]) + b"abc" + self.SOLC, @@ -126,12 +131,40 @@ def test_solc_is_required_and_one_hash_at_most(self): def test_shapes_solc_emits_are_metadata(self): self.assert_metadata(bytes([0xA2]) + self.IPFS + self.SOLC, "ipfs + release solc") self.assert_metadata(bytes([0xA1]) + self.SOLC, "bytecodeHash none") - prerelease = bytes([0x64]) + b"solc" + bytes([0x78, 0x18]) + b"0.8.29-nightly.2025.1.1+" - self.assert_metadata(bytes([0xA2]) + self.IPFS + prerelease, "prerelease solc") bzzr1 = bytes([0x65]) + b"bzzr1" + bytes([0x58, 0x20]) + bytes(32) experimental = bytes([0x6C]) + b"experimental" + bytes([0xF5]) self.assert_metadata(bytes([0xA3]) + bzzr1 + experimental + self.SOLC, "bzzr1 + experimental") + # Round 3: the solc version must be the one foundry.toml pins. + + def test_reviewer_counterexample_with_an_impossible_version_is_compared_in_full(self): + # {"solc": h'ffffff'} plus its length. Shaped like release metadata, but + # the pinned compiler can't write version 255.255.255, and these bytes + # execute (LOG1/PUSH5/SELFDESTRUCT/STOP). + full = "600060006000a164736f6c6343ffffff000a" + self.assertEqual(code("0x" + full), full) + self.assertNotEqual(code("0x" + full), code("0x600060006000")) + + def test_other_solc_versions_are_not_metadata(self): + major, minor, patch = PINNED_SOLC + for why, version in { + "previous patch": bytes([major, minor, (patch - 1) % 256]), + "next patch": bytes([major, minor, (patch + 1) % 256]), + "next minor": bytes([major, (minor + 1) % 256, patch]), + }.items(): + solc = bytes([0x64]) + b"solc" + bytes([0x43]) + version + self.assert_not_metadata(bytes([0xA2]) + self.IPFS + solc, why) + + def test_prerelease_solc_is_not_metadata(self): + prerelease = bytes([0x64]) + b"solc" + bytes([0x78, 0x18]) + b"0.8.29-nightly.2025.1.1+" + self.assert_not_metadata(bytes([0xA2]) + self.IPFS + prerelease, "prerelease solc") + + def test_solc_version_bytes(self): + self.assertEqual(solc_version_bytes("0.8.28"), bytes([0, 8, 28])) + for bad in ("0.8", "^0.8.28", "0.8.28-nightly", "0.8.256", "v0.8.28", ""): + with self.assertRaises(ValueError, msg=bad): + solc_version_bytes(bad) + def test_empty_and_tiny_code(self): self.assertEqual(code("0x"), "") self.assertEqual(code("0x00"), "00") From 36f0ea1c280470170eb95588885227523f93dce1 Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Mon, 2 Nov 2026 16:27:17 +0000 Subject: [PATCH 5/6] fix: compare contract bytecode in full; check out submodules recursively (V2-1750) Review of 7290274: no shape or version check can prove a tail is solc metadata rather than code. {"solc": 0.8.28} (a164736f6c634300081c000a) also executes (LOG1), so stripping it let different bytecode compare equal. forge's rawMetadata does not hash to the tail either, so the tail can't be authenticated against it. The metadata only ever differed because contracts.yml checked out submodules non-recursively: Foundry then misses the remappings it auto-detects from OpenZeppelin's nested submodules (erc4626-tests, halmos-cheatcodes), and the remappings are hashed into the metadata. With a recursive checkout, a fresh forge 1.7.1 build reproduces artifacts/PaymentVaultV2.json byte for byte, metadata included (checked on Linux; the flat checkout differs only in the metadata hash). So contracts.yml now checks out recursively and the checker compares the full bytecode, stripping nothing. The CBOR parser and its tests are gone; the failure message points at recursive submodules when only the metadata differs. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_contract_artifacts.py | 162 ++-------------- .../scripts/test_check_contract_artifacts.py | 182 ------------------ .github/workflows/contracts.yml | 9 +- 3 files changed, 17 insertions(+), 336 deletions(-) delete mode 100644 .github/scripts/test_check_contract_artifacts.py diff --git a/.github/scripts/check_contract_artifacts.py b/.github/scripts/check_contract_artifacts.py index 88ff7b7..8beb790 100644 --- a/.github/scripts/check_contract_artifacts.py +++ b/.github/scripts/check_contract_artifacts.py @@ -11,173 +11,34 @@ artifacts/PaymentVaultV2.json ABI, bytecode and deployedBytecode abi/IPaymentVault.json ABI (it is PaymentVaultV2's ABI) -Bytecode is compared without the trailing CBOR metadata. That blob hashes the -compiler settings, including the remappings Foundry auto-detects from whatever -nested submodules happen to be checked out, so it differs between otherwise -identical builds. The code before it is deterministic for a pinned solc. - -The tail is only stripped when it really is solc metadata: the last two bytes -give a length, and the bytes before them must be exactly one well-formed CBOR -map of that length, shaped like solc's (unique text keys, each value of the -type and length solc writes; see HASH_LENGTHS below) and naming the compiler -foundry.toml pins (`solc_version`) as its `solc` version. Anything else (no -metadata, a truncated or oversized footer, non-CBOR bytes, a map that merely -looks like metadata, another compiler version) is compared as full bytecode, -so a check can't pass by stripping executable code. +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 re import sys -import tomllib from pathlib import Path COMMITTED = Path("artifacts/PaymentVaultV2.json") COMMITTED_ABI = Path("abi/IPaymentVault.json") -FOUNDRY_TOML = Path(__file__).resolve().parents[2] / "foundry.toml" - - -def solc_version_bytes(version): - """The 3 bytes solc writes as `solc` in its metadata for release `x.y.z`. - - Raises ValueError for anything else (ranges, prereleases, components > 255). - """ - match = re.fullmatch(r"(\d+)\.(\d+)\.(\d+)", version) - if not match or any(int(part) > 255 for part in match.groups()): - raise ValueError(f"not a release solc version: {version!r}") - return bytes(int(part) for part in match.groups()) - - -def pinned_solc(): - """foundry.toml's [profile.default] solc_version, as solc writes it in metadata.""" - config = tomllib.loads(FOUNDRY_TOML.read_text()) - version = config.get("profile", {}).get("default", {}).get("solc_version") - if not isinstance(version, str): - sys.exit(f"{FOUNDRY_TOML}: [profile.default] solc_version is not set") - try: - return solc_version_bytes(version) - except ValueError as e: - sys.exit(f"{FOUNDRY_TOML}: {e}") - - -# The compiler the artifacts are built with. A metadata tail naming any other -# version can't come from this build, so it is not stripped. -PINNED_SOLC = pinned_solc() def canonical(value): return json.dumps(value, sort_keys=True) -# What solc writes into the metadata map. Each key appears at most once, as a -# CBOR text string, with exactly this value shape: -# ipfs byte string, 34 bytes (sha2-256 multihash) -# bzzr0, bzzr1 byte string, 32 bytes -# solc byte string, 3 bytes (release x.y.z); must be PINNED_SOLC. -# (A prerelease solc writes a text string here instead; the -# pin is a release, so that is rejected too.) -# experimental true -# `solc` is required: every build that appends a metadata map includes it. -HASH_LENGTHS = {"ipfs": 34, "bzzr0": 32, "bzzr1": 32} - - -def _cbor_item(data, pos): - """Parse one CBOR item of the kinds solc metadata uses. - - Returns ((kind, value), next_pos), or None if `data` doesn't hold such an - item at `pos`. Kinds: "uint", "bytes", "text", "bool", and "map", whose - value is a list of (key, value) pairs in order (duplicates kept, so the - caller can reject them). - """ - if pos >= len(data): - return None - major, info = data[pos] >> 5, data[pos] & 0x1F - pos += 1 - if major == 7: - return (("bool", info == 21), pos) if info in (20, 21) else None - if info < 24: - arg = info - elif info in (24, 25): - width = 1 if info == 24 else 2 - if pos + width > len(data): - return None - arg = int.from_bytes(data[pos:pos + width], "big") - pos += width - else: - return None - if major == 0: - return ("uint", arg), pos - if major in (2, 3): - if pos + arg > len(data): - return None - raw = bytes(data[pos:pos + arg]) - if major == 2: - return ("bytes", raw), pos + arg - try: - return ("text", raw.decode("utf-8")), pos + arg - except UnicodeDecodeError: - return None - if major == 5: - pairs = [] - for _ in range(arg): - key = _cbor_item(data, pos) - if key is None: - return None - value = _cbor_item(data, key[1]) - if value is None: - return None - pairs.append((key[0], value[0])) - pos = value[1] - return ("map", pairs), pos - return None - - -def _is_solc_metadata(pairs): - """True only for a map shaped exactly like solc's metadata (see above).""" - seen = set() - for (key_kind, key), (kind, value) in pairs: - if key_kind != "text" or key in seen: - return False - seen.add(key) - if key in HASH_LENGTHS: - ok = kind == "bytes" and len(value) == HASH_LENGTHS[key] - elif key == "solc": - ok = kind == "bytes" and value == PINNED_SOLC - elif key == "experimental": - ok = kind == "bool" and value - else: - ok = False - if not ok: - return False - return "solc" in seen and len(seen & HASH_LENGTHS.keys()) <= 1 - - -def metadata_length(raw): - """Length in bytes of solc's metadata tail (CBOR map + 2-byte length), or 0.""" - if len(raw) < 3: - return 0 - cbor_len = int.from_bytes(raw[-2:], "big") - if cbor_len == 0 or cbor_len + 2 > len(raw): - return 0 - blob = raw[-2 - cbor_len:-2] - parsed = _cbor_item(blob, 0) - if parsed is None or parsed[1] != len(blob): - return 0 # malformed, or doesn't fill exactly the stated length - (kind, pairs), _ = parsed - if kind != "map" or not _is_solc_metadata(pairs): - return 0 - return cbor_len + 2 - - def code(entry): - """Hex code without 0x and without solc's metadata tail, when there is one.""" + """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 - raw = bytes.fromhex(hex_code.removeprefix("0x")) - tail = metadata_length(raw) - return (raw[:-tail] if tail else raw).hex() + return hex_code.removeprefix("0x").lower() def main(): @@ -203,7 +64,10 @@ def main(): print( "Regenerate with `forge build`, copy " "artifacts/PaymentVaultV2.sol/PaymentVaultV2.json over " - f"{COMMITTED}, and write its `abi` array to {COMMITTED_ABI}." + 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/") diff --git a/.github/scripts/test_check_contract_artifacts.py b/.github/scripts/test_check_contract_artifacts.py deleted file mode 100644 index 252cecf..0000000 --- a/.github/scripts/test_check_contract_artifacts.py +++ /dev/null @@ -1,182 +0,0 @@ -#!/usr/bin/env python3 -"""Tests for check_contract_artifacts.py's metadata stripping. - -Run: python3 .github/scripts/test_check_contract_artifacts.py (contracts.yml -does, before the artifact comparison). -""" - -import json -import sys -import unittest -from pathlib import Path - -sys.path.insert(0, str(Path(__file__).resolve().parent)) - -from check_contract_artifacts import ( # noqa: E402 - PINNED_SOLC, - code, - metadata_length, - solc_version_bytes, -) - -REPO = Path(__file__).resolve().parents[2] - - -def solc_tail(ipfs_byte=0x11, version=PINNED_SOLC): - """A metadata tail shaped like solc's: {ipfs: <34 bytes>, solc: <3 bytes>} + length.""" - blob = ( - bytes([0xA2]) - + bytes([0x64]) + b"ipfs" + bytes([0x58, 0x22]) + bytes([ipfs_byte]) * 34 - + bytes([0x64]) + b"solc" + bytes([0x43]) + bytes(version) - ) - return blob + len(blob).to_bytes(2, "big") - - -EXECUTABLE = bytes.fromhex("6080604052348015600e575f5ffd5b50") - - -class MetadataStripping(unittest.TestCase): - def test_strips_a_solc_shaped_tail(self): - raw = EXECUTABLE + solc_tail() - self.assertEqual(metadata_length(raw), len(solc_tail())) - self.assertEqual(code(raw.hex()), EXECUTABLE.hex()) - - def test_builds_differing_only_in_metadata_compare_equal(self): - a = "0x" + (EXECUTABLE + solc_tail(ipfs_byte=0x11)).hex() - b = "0x" + (EXECUTABLE + solc_tail(ipfs_byte=0x22)).hex() - self.assertEqual(code(a), code(b)) - - def test_different_code_with_the_same_tail_still_differs(self): - tail = solc_tail() - a = "0x" + (bytes.fromhex("600160005500") + tail).hex() - b = "0x" + (bytes.fromhex("600260005500") + tail).hex() - self.assertNotEqual(code(a), code(b)) - - def test_metadata_free_code_is_compared_in_full(self): - # Hermes's reproduction: the old parser read 0x5500 as a length and - # stripped everything, so these compared equal. - self.assertEqual(code("0x600160005500"), "600160005500") - self.assertNotEqual(code("0x600160005500"), code("0x600260005500")) - - def test_oversized_footer_is_not_stripped(self): - raw = EXECUTABLE + bytes([0xFF, 0xFF]) - self.assertEqual(metadata_length(raw), 0) - self.assertEqual(code(raw.hex()), raw.hex()) - - def test_truncated_tail_is_not_stripped(self): - # Drop bytes from the front of a valid tail: the footer now claims more - # CBOR than is left before it. - raw = solc_tail()[10:] - self.assertEqual(metadata_length(raw), 0) - - def test_footer_over_non_cbor_bytes_is_not_stripped(self): - raw = EXECUTABLE + bytes([0x60, 0x01, 0x55]) + (3).to_bytes(2, "big") - self.assertEqual(metadata_length(raw), 0) - - def test_map_shorter_than_the_footer_is_not_stripped(self): - blob = bytes([0xA1, 0x64]) + b"solc" + bytes([0x43]) + PINNED_SOLC - padded = blob + bytes([0x00]) # one byte of garbage inside the stated length - raw = EXECUTABLE + padded + len(padded).to_bytes(2, "big") - self.assertEqual(metadata_length(raw), 0) - - def test_unknown_keys_are_not_metadata(self): - blob = bytes([0xA1, 0x63]) + b"foo" + bytes([0x01]) - raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") - self.assertEqual(metadata_length(raw), 0) - - # Round 2: maps that are well-formed CBOR but not solc metadata. - - def test_reviewer_counterexample_is_compared_in_full(self): - # {"solc": 1} plus its length. Valid CBOR with a whitelisted key, but solc - # writes a 3-byte version, and these bytes also execute (LOG1/PUSH5/STOP). - self.assertEqual(code("0x600060006000a164736f6c63010007"), "600060006000a164736f6c63010007") - self.assertNotEqual(code("0x600060006000a164736f6c63010007"), code("0x600060006000")) - - def assert_not_metadata(self, blob, why): - raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") - self.assertEqual(metadata_length(raw), 0, why) - - def assert_metadata(self, blob, why): - raw = EXECUTABLE + blob + len(blob).to_bytes(2, "big") - self.assertEqual(metadata_length(raw), len(blob) + 2, why) - self.assertEqual(code(raw.hex()), EXECUTABLE.hex(), why) - - SOLC = bytes([0x64]) + b"solc" + bytes([0x43]) + PINNED_SOLC - IPFS = bytes([0x64]) + b"ipfs" + bytes([0x58, 0x22]) + bytes([0x11]) * 34 - - def test_duplicate_keys_are_not_metadata(self): - self.assert_not_metadata(bytes([0xA2]) + self.SOLC + self.SOLC, "duplicate solc") - - def test_byte_string_keys_are_not_metadata(self): - key_as_bytes = bytes([0x44]) + b"solc" + bytes([0x43]) + PINNED_SOLC - self.assert_not_metadata(bytes([0xA1]) + key_as_bytes, "byte-string key") - - def test_wrong_value_types_and_lengths_are_not_metadata(self): - cases = { - "solc as uint": bytes([0xA1, 0x64]) + b"solc" + bytes([0x01]), - "solc 2 bytes": bytes([0xA1, 0x64]) + b"solc" + bytes([0x42]) + PINNED_SOLC[:2], - "solc empty text": bytes([0xA1, 0x64]) + b"solc" + bytes([0x60]), - "ipfs 33 bytes": bytes([0xA2, 0x64]) + b"ipfs" + bytes([0x58, 0x21]) + bytes(33) + self.SOLC, - "ipfs as text": bytes([0xA2, 0x64]) + b"ipfs" + bytes([0x63]) + b"abc" + self.SOLC, - "experimental false": bytes([0xA2, 0x6C]) + b"experimental" + bytes([0xF4]) + self.SOLC, - } - for why, blob in cases.items(): - self.assert_not_metadata(blob, why) - - def test_solc_is_required_and_one_hash_at_most(self): - self.assert_not_metadata(bytes([0xA1]) + self.IPFS, "no solc") - bzzr1 = bytes([0x65]) + b"bzzr1" + bytes([0x58, 0x20]) + bytes(32) - self.assert_not_metadata(bytes([0xA3]) + self.IPFS + bzzr1 + self.SOLC, "two hashes") - - def test_shapes_solc_emits_are_metadata(self): - self.assert_metadata(bytes([0xA2]) + self.IPFS + self.SOLC, "ipfs + release solc") - self.assert_metadata(bytes([0xA1]) + self.SOLC, "bytecodeHash none") - bzzr1 = bytes([0x65]) + b"bzzr1" + bytes([0x58, 0x20]) + bytes(32) - experimental = bytes([0x6C]) + b"experimental" + bytes([0xF5]) - self.assert_metadata(bytes([0xA3]) + bzzr1 + experimental + self.SOLC, "bzzr1 + experimental") - - # Round 3: the solc version must be the one foundry.toml pins. - - def test_reviewer_counterexample_with_an_impossible_version_is_compared_in_full(self): - # {"solc": h'ffffff'} plus its length. Shaped like release metadata, but - # the pinned compiler can't write version 255.255.255, and these bytes - # execute (LOG1/PUSH5/SELFDESTRUCT/STOP). - full = "600060006000a164736f6c6343ffffff000a" - self.assertEqual(code("0x" + full), full) - self.assertNotEqual(code("0x" + full), code("0x600060006000")) - - def test_other_solc_versions_are_not_metadata(self): - major, minor, patch = PINNED_SOLC - for why, version in { - "previous patch": bytes([major, minor, (patch - 1) % 256]), - "next patch": bytes([major, minor, (patch + 1) % 256]), - "next minor": bytes([major, (minor + 1) % 256, patch]), - }.items(): - solc = bytes([0x64]) + b"solc" + bytes([0x43]) + version - self.assert_not_metadata(bytes([0xA2]) + self.IPFS + solc, why) - - def test_prerelease_solc_is_not_metadata(self): - prerelease = bytes([0x64]) + b"solc" + bytes([0x78, 0x18]) + b"0.8.29-nightly.2025.1.1+" - self.assert_not_metadata(bytes([0xA2]) + self.IPFS + prerelease, "prerelease solc") - - def test_solc_version_bytes(self): - self.assertEqual(solc_version_bytes("0.8.28"), bytes([0, 8, 28])) - for bad in ("0.8", "^0.8.28", "0.8.28-nightly", "0.8.256", "v0.8.28", ""): - with self.assertRaises(ValueError, msg=bad): - solc_version_bytes(bad) - - def test_empty_and_tiny_code(self): - self.assertEqual(code("0x"), "") - self.assertEqual(code("0x00"), "00") - - def test_committed_artifact_has_a_recognised_tail(self): - artifact = json.loads((REPO / "artifacts" / "PaymentVaultV2.json").read_text()) - for key in ("bytecode", "deployedBytecode"): - entry = artifact[key] - hex_code = entry["object"] if isinstance(entry, dict) else entry - raw = bytes.fromhex(hex_code.removeprefix("0x")) - self.assertGreater(metadata_length(raw), 0, key) - - -if __name__ == "__main__": - unittest.main() diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index c07d234..7cb0ffe 100644 --- a/.github/workflows/contracts.yml +++ b/.github/workflows/contracts.yml @@ -13,7 +13,6 @@ on: - "foundry.toml" - "remappings.txt" - ".github/scripts/check_contract_artifacts.py" - - ".github/scripts/test_check_contract_artifacts.py" - ".github/workflows/contracts.yml" permissions: @@ -26,14 +25,14 @@ jobs: steps: - uses: actions/checkout@v4 with: - submodules: true + # 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 - - name: Test the artifact checker - run: python3 .github/scripts/test_check_contract_artifacts.py - - name: Build contracts run: forge build --out "$RUNNER_TEMP/forge-out" --cache-path "$RUNNER_TEMP/forge-cache" From 93bb12c8e51d2f289a8767d32f0fd9ed39d133d3 Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Tue, 3 Nov 2026 09:08:49 +0000 Subject: [PATCH 6/6] ci: pin Foundry to v1.8.7 for the contract artifacts check (V2-1750) Follow-up to the approval of 36f0ea1: the byte-for-byte check ran on whatever `stable` Foundry was that day (1.8.7; 1.9.0 shipped since). Pin the version the check passed on, so a Foundry release can't change the build under it. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/contracts.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index 7cb0ffe..098b378 100644 --- a/.github/workflows/contracts.yml +++ b/.github/workflows/contracts.yml @@ -32,6 +32,11 @@ jobs: - 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"