Skip to content

fix!: enforce merkle payment amount verification#57

Closed
mickvandijke wants to merge 6 commits intomainfrom
fix/merkle-underpayment-verification
Closed

fix!: enforce merkle payment amount verification#57
mickvandijke wants to merge 6 commits intomainfrom
fix/merkle-underpayment-verification

Conversation

@mickvandijke
Copy link
Copy Markdown
Collaborator

Summary

  • Fix critical vulnerability where merkle payment verifier only checked paid_amount.is_zero() instead of paid_amount >= candidate.price
  • A malicious client could submit fake low prices in PoolCommitment candidates while keeping the real poolHash, causing the contract to charge almost nothing while nodes still accepted the proof
  • Replace the zero-check with paid_amount < node.price so each paid candidate must receive at least their ML-DSA-65 signed quoted price
  • Add test_merkle_underpayment_rejected unit test
  • Fix 3 existing unit tests missing the Amount field in paid_node_addresses tuples

The attack vector (now closed)

  1. Client collects real candidates with real ML-DSA-65 signed prices
  2. Computes real poolHash (commits to full candidate data including signatures)
  3. Constructs PoolCommitment with real poolHash but fake 1-wei prices
  4. Contract calculates median(1 wei) × 2^depth — essentially nothing
  5. Node accepted because it only checked > 0, not >= quoted price

Note

MerklePaymentCandidatePool::verify_prices() in evmlib already identified this gap but was never called. The node-side fix is simpler and sufficient since the proof already contains both the on-chain paid amounts and the ML-DSA-65 signed candidate prices.

Test plan

  • All 40 verifier unit tests pass (cargo test --lib -- payment::verifier::tests)
  • Legitimate merkle payments still work (verified via ant-client E2E tests)
  • Full CI

🤖 Generated with Claude Code

mickvandijke and others added 6 commits April 1, 2026 19:36
The merkle payment verifier only checked that paid amounts were non-zero,
not that they met the candidate's quoted price. A malicious client could
submit fake low prices in PoolCommitment candidates while keeping the real
poolHash, causing the contract to charge almost nothing while nodes still
accepted the proof.

Replace `paid_amount.is_zero()` with `paid_amount < node.price` so each
paid candidate must receive at least their ML-DSA-65 signed quoted price.
Also fix existing unit tests that were missing the Amount field in
paid_node_addresses tuples, and add test_merkle_underpayment_rejected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Migrate from the old QuotingMetrics-based pricing and split
DataPayments/MerklePayments contracts to the unified PaymentVault API
in evmlib. Key changes:

- Replace QuotingMetrics with a single `price: Amount` field on quotes
- Replace logarithmic pricing with simple quadratic formula (n/6000)²
- Unify data_payments_address + merkle_payments_address into
  payment_vault_address
- Verify payments via completedPayments mapping instead of
  verify_data_payment batch call

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The verifier checked `paid_amount >= node.price` (individual quote) but
the contract pays each winner `median16(quotes) * 2^depth / depth`. A
winner quoting above the median could be paid less than their quote,
causing the node to incorrectly reject a valid payment.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Integrate `SingleNodePayment::from_quotes` to derive correct on-chain payment amounts. This ensures exact-match checks in the contract's `verifyPayment` function pass by reconstructing amounts as used by the client.
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.

1 participant