Skip to content

fix(merkle-allowlist): emit sha256 Soroban-compatible proofs instead of keccak256 - #448

Merged
kilodesodiq-arch merged 1 commit into
ChainForgee:mainfrom
ibrahimmosouf-png:fix/issue-425-merkle-allowlist-hash
Aug 20, 2026
Merged

fix(merkle-allowlist): emit sha256 Soroban-compatible proofs instead of keccak256#448
kilodesodiq-arch merged 1 commit into
ChainForgee:mainfrom
ibrahimmosouf-png:fix/issue-425-merkle-allowlist-hash

Conversation

@ibrahimmosouf-png

Copy link
Copy Markdown
Contributor

Summary

Closes #425

tools/merkle-allowlist built EVM-style proofs (keccak256 + abi.encodePacked(address, amount) via ethers/merkletreejs) for a verifier the Soroban aid_escrow contract does not run, so every root and proof it emitted failed on-chain with Error::InvalidProof while printing "valid" locally — a false-positive smoke signal. This PR rewrites the tool on Node's built-in crypto SHA-256 with the contract's exact leaf (sha256(<canonical Address string>)) and sorted-pair combine, drops the EVM dependency stack entirely, and pins the JS↔Rust agreement with a new contract-side compatibility test suite that recomputes the tool's root and claims on-chain with its proofs.

The single most important design decision: the canonical encoding is now defined in one place (the tool README, mirrored by doc comments) and enforced bidirectionally — the Rust test recomputes the tool's committed root and proofs from sample_allowlist.json, so any drift on either side fails CI.

Why

The old tool's leaf was keccak256(abi.encodePacked(address, amount)) — a keccak hash over address and amount — while verify_merkle_proof_for_claimant recomputes sha256(claimant.to_string()) binding only the address. Three independent incompatibilities (hash function, leaf payload, address casing/encoding) meant every generated proof was rejected on-chain. The tool even shipped an "on-chain verification" path wired to ethers.JsonRpcProvider + MERKLE_CONTRACT_ADDRESS/MERKLE_CONTRACT_ABI_PATH, which cannot talk to a Soroban contract. The fix is not a dependency swap: it replicates the contract's exact byte-level algorithm in JS and proves equivalence with on-chain tests.

The amount-binding question is deliberately left untouched (it is a separate open issue, per the issue's own scope): the tool now emits the address-only leaf the contract actually verifies and documents that binding amounts is a future, coordinated change.

What was built

tools/merkle-allowlist/:

File What it contains
index.js Rewritten zero-dependency implementation: makeLeaf = sha256(addressString) (matches hash_address byte-for-byte), hashPair = sha256(left || right) with left <= right byte-wise, buildRoot/buildProof with odd-leaf promotion, and a verify walk identical to verify_merkle_proof_for_claimant. Emits the root plus the valid, invalid_proof_path, wrong_recipient, wrong_leaf, and mismatched_root scenarios with stable result objects, plus a per-entry proof dump.
package.json ethers, merkletreejs, keccak256 removed; zero runtime dependencies (engines.node >= 16).
sample_allowlist.json Deterministic test-env addresses chosen from generator slots the test harness never consumes (see test file doc comment), so the Rust compat test and the tool share one canonical allowlist.
README.md Documents the exact leaf byte string, pair-combine, root/proof layout, hex format, and metadata keys — the contract of record for keeping Rust and JS in sync.

New test file (matching test): app/onchain/contracts/aid_escrow/tests/merkle_allowlist_tool.rs — 5 tests:

  • rust_encoding_matches_tool_output — recomputes the root and proof for sample_allowlist.json and asserts they equal the tool's committed output (the sync pin).
  • claim_with_proof_accepts_tool_format_proofs — every allowlist entry claims a real merkle-protected package on-chain with tool-format proofs and receives the payout.
  • wrong_recipient_proof_is_rejected — a stranger presenting an allowlisted entry's proof fails with InvalidProof; a direct (proof-less) claim on a merkle package also fails.
  • tampered_proof_is_rejected — flipping one bit of a sibling in the proof fails verification.
  • mismatched_root_is_rejected — a root from a reordered tree is rejected even with a proof valid against the real root.

Acceptance criteria coverage

  • node index.js generates a root and proof that verify_merkle_proof_for_claimant (sha256, sorted-pair) accepts for a known leaf; a test or smoke script demonstrates on-chain acceptance. (tests/merkle_allowlist_tool.rsrust_encoding_matches_tool_output pins the exact hashes; claim_with_proof_accepts_tool_format_proofs performs real on-chain claims with those proofs.)
  • The valid, invalid_proof_path, wrong_recipient, wrong_leaf, and mismatched_root scenarios all behave correctly against the Soroban leaf encoding. (Tool output: validsuccess: true; the other four → the documented error codes; negative cases mirrored on-chain in the Rust tests.)
  • ethers, merkletreejs, and keccak256 are removed from tools/merkle-allowlist/package.json. (package.json now has no dependencies field.)
  • tools/merkle-allowlist/README.md documents the exact leaf byte string and hash/pairing algorithm so Rust and JS stay in sync. (New "Canonical encoding" table + "Previous EVM implementation" section explaining why the old format is gone.)

Deliberately deferred

  • Amount-bound leaves — binding amount (or package_id) into the leaf is a separate open issue per the issue's scope; the tool now emits the contract's actual address-only leaf and the README names the dependency explicitly rather than silently encoding amounts.
  • maybeCallOnchain Soroban equivalent — the old EVM on-chain verification path is removed (it targeted an Ethereum JSON-RPC endpoint and cannot talk to Soroban). A real on-chain smoke script is out of scope; the Rust test suite provides the on-chain acceptance proof instead.

Test plan

  • cargo test --package aid_escrow --test merkle_allowlist_tool — 5/5 passing (5 new)
  • cargo test --package aid_escrow — 193/193 passing (0 failures; 5 new tests for this feature)
  • cargo fmt --all -- --check — clean
  • cargo clippy --tests --target x86_64-unknown-linux-gnu — 0 warnings/errors
  • cargo clippy --target wasm32-unknown-unknown — 0 warnings/errors
  • node index.js (in tools/merkle-allowlist) — all 5 scenarios behave correctly

Env vars / Notes

No new environment variables. The previous TESTNET_RPC_URL / MERKLE_CONTRACT_ADDRESS / MERKLE_CONTRACT_ABI_PATH variables and the npm ci install step are gone — the tool is dependency-free and offline. Any root previously published from the old tool is invalid under the corrected encoding and must be regenerated; the contract-side verifier is unchanged, so no contract migration is involved.

…of keccak256

The tool built EVM-style keccak256(abi.encodePacked(address, amount))
leaves for an Ethereum verifier, so every proof it produced failed
on-chain with Error::InvalidProof while printing "valid" locally.
Rewrite it on Node's crypto sha256 with the contract's exact leaf
(sha256 of the canonical Address string) and sorted-pair combine, and
drop ethers/merkletreejs/keccak256. Pin the agreement with a Rust
compat test that recomputes the root and claims on-chain with the
tool's proofs.

@kilodesodiq-arch kilodesodiq-arch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@kilodesodiq-arch
kilodesodiq-arch merged commit 380316f into ChainForgee:main Aug 20, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

merkle-allowlist tool emits keccak256 Ethereum proofs: incompatible with claim_with_proof SHA-256 verification

2 participants