feat!: adapt to evmlib PaymentVault API and simplify pricing#56
feat!: adapt to evmlib PaymentVault API and simplify pricing#56mickvandijke wants to merge 14 commits intomainfrom
Conversation
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>
…-payment-vault-v2 branch
Add unit and e2e tests covering the remaining Section 18 scenarios: Unit tests (32 new): - Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round dual-evidence, #28 dynamic threshold undersized, #33 batched per-key, #34 partial response unresolved, #42 quorum-derived paid-list auth - Admission: #5 unauthorized peer, #7 out-of-range rejected - Config: #18 invalid config rejected, #26 dynamic paid threshold - Scheduling: #8 dedup safety, #8 replica/paid collapse - Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion, #38 snapshot stability mid-join, #39 unreachable removal + slot fill, #40 cooldown peer removed, #41 cycle termination guarantee, consecutive rounds, cycle preserves sync times - Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset on heal, #52 paid/record timestamps independent, #23 entry removal - Audit: #19/#53 partial failure mixed responsibility, #54 all pass, #55 empty failure discard, #56 repair opportunity filter, response count validation, digest uses full record bytes - Types: #13 bootstrap drain, repair opportunity edge cases, terminal state variants - Bootstrap claims: #46 first-seen recorded, #49 cleared on normal E2e tests (4 new): - #2 fresh offer with empty PoP rejected - #5/#37 neighbor sync request returns response - #11 audit challenge multi-key (present + absent) - Fetch not-found for non-existent key 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.
There was a problem hiding this comment.
Pull request overview
Updates ant-node to match the refactored evmlib PaymentVault API by removing QuotingMetrics from quotes/candidate nodes, simplifying pricing to a quadratic function, and adjusting on-chain verification to the new contract surface.
Changes:
- Replace
QuotingMetricswith a singleprice: Amountacross quoting + merkle candidate flows, updating signing/verification and tests accordingly. - Switch payment verification to PaymentVault APIs (
verifyPayment,completedPayments) and update merkle verification to read completed merkle payment info + validate paid amounts. - Replace the previous pricing logic with a U256-based quadratic pricing formula and update devnet manifest wiring to a unified
payment_vault_address.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/payment/verifier.rs |
Adapts EVM + merkle payment verification to the unified PaymentVault API and new on-chain data structures. |
src/payment/single_node.rs |
Reworks SingleNode verification to use completedPayments and removes QuotingMetrics plumbing. |
src/payment/quote.rs |
Generates quotes/candidate nodes using price (derived from record count) and updates signature bytes accordingly. |
src/payment/pricing.rs |
Implements the new quadratic pricing formula in wei using Amount arithmetic and updates tests. |
src/payment/proof.rs |
Updates proof tests to construct quotes/candidates with price instead of QuotingMetrics. |
src/storage/handler.rs |
Adjusts merkle candidate quote request test expectations for the new candidate node structure. |
tests/e2e/merkle_payment.rs |
Updates E2E merkle payment test helpers to build/sign candidate nodes with price. |
src/devnet.rs |
Unifies devnet manifest EVM contract address fields into payment_vault_address. |
src/bin/ant-devnet/main.rs |
Emits unified vault address into the devnet manifest from the Anvil network config. |
Cargo.toml |
Points evmlib dependency to the refactor branch to pick up the new PaymentVault API. |
Cargo.lock |
Locks the new git-sourced evmlib resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //! Uses the formula `(close_records_stored / 6000)^2` to calculate storage price. | ||
| //! Integer division means nodes with fewer than 6000 records get a ratio of 0, | ||
| //! but a minimum floor of 1 prevents free storage. |
There was a problem hiding this comment.
The module docs claim that “integer division means nodes with fewer than 6000 records get a ratio of 0”, but calculate_price() scales before dividing (n² * 1e18 / 6000²), so prices for 1..5999 records are non-zero (and only n=0 hits the 1 wei floor). Please update the documentation to match the implemented formula/behavior.
src/payment/verifier.rs
Outdated
| let total_quotes = single_payment.quotes.len(); | ||
| let mut valid_paid_count: usize = 0; | ||
|
|
||
| for result in &results { | ||
| if result.isValid && result.amountPaid > Amount::ZERO { | ||
| valid_paid_count += 1; | ||
| } | ||
| } | ||
|
|
||
| if paid_results.is_empty() { | ||
| if valid_paid_count == 0 { | ||
| let xorname_hex = hex::encode(xorname); | ||
| return Err(Error::Payment(format!( | ||
| "Payment verification failed on-chain for {xorname_hex} (no paid quotes found)" | ||
| "Payment verification failed on-chain for {xorname_hex}: \ | ||
| no valid paid quotes found ({total_quotes} checked)" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
verify_evm_payment() no longer rejects the case where a quote is paid on-chain (amountPaid > 0) but marked invalid (isValid == false). Currently, the code only checks that at least one result is both paid and valid, which would incorrectly accept a payment set where one paid quote is invalid as long as another is valid. Please restore strictness by failing if any paid result is invalid (while still allowing unpaid quotes to be invalid).
| candidate_prices.sort_unstable(); // ascending | ||
| // Upper median (index 8 of 16) — matches Solidity's median16 (k = 8) | ||
| let median_price = candidate_prices[candidate_prices.len() / 2]; | ||
| let total_amount = median_price * Amount::from(1u64 << payment_info.depth); |
There was a problem hiding this comment.
1u64 << payment_info.depth can overflow/panic (or invoke undefined behavior in release builds) if depth >= 64. Even if current contract depths are small, it’s safer to compute 2^depth using checked shifting (e.g., checked_shl) and return a verification error on overflow, or use an Amount/U256-based shift that cannot overflow for supported depths.
| let total_amount = median_price * Amount::from(1u64 << payment_info.depth); | |
| let shift = u32::from(payment_info.depth); | |
| let factor = 1u64 | |
| .checked_shl(shift) | |
| .ok_or_else(|| { | |
| Error::Payment(format!( | |
| "Payment depth too large for shift: depth={}", | |
| payment_info.depth | |
| )) | |
| })?; | |
| let total_amount = median_price * Amount::from(factor); |
| // Node-calculated price based on records stored | ||
| assert!(candidate.price >= evmlib::common::Amount::ZERO); |
There was a problem hiding this comment.
This assertion is effectively a no-op: candidate.price can never be negative, so >= Amount::ZERO will always pass. Since create_test_protocol() initializes QuotingMetricsTracker::new(1000, 100), you can assert an exact expected price (e.g., candidate.price == calculate_price(100)) to actually validate the new pricing-based candidate construction.
| // Node-calculated price based on records stored | |
| assert!(candidate.price >= evmlib::common::Amount::ZERO); | |
| // Node-calculated price based on records stored must be positive | |
| assert!(candidate.price > evmlib::common::Amount::ZERO); |
|
|
||
| # Payment verification - autonomi network lookup + EVM payment | ||
| evmlib = "0.5.0" | ||
| evmlib = { git = "https://github.com/WithAutonomi/evmlib/", branch = "refactor/unify-payment-vault-v2" } |
There was a problem hiding this comment.
evmlib is pulled from a moving git branch. For reproducible builds and supply-chain hygiene, prefer a published crate version or pin to an immutable git rev in Cargo.toml (not just in Cargo.lock). The PR description also notes this must be updated before merge.
| evmlib = { git = "https://github.com/WithAutonomi/evmlib/", branch = "refactor/unify-payment-vault-v2" } | |
| evmlib = { git = "https://github.com/WithAutonomi/evmlib/", rev = "0123456789abcdef0123456789abcdef01234567" } |
Nodes should store as many chunks as disk allows. The LMDB map size (db_size_gb) remains as the only storage bound. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Nodes no longer advertise or track a maximum record count. The QuotingMetricsTracker API is simplified to only take initial_records. The evmlib QuotingMetrics::max_records field is set to 0 since we cannot remove it from the external type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/payment/verifier.rs
Outdated
|
|
||
| for result in &results { | ||
| if result.isValid && result.amountPaid > Amount::ZERO { | ||
| valid_paid_count += 1; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The on-chain verification currently only checks that at least one result is both isValid and has a non-zero amountPaid. This can incorrectly pass if a quote is paid (amountPaid > 0) but isValid is false (e.g., wrong expected amount), since that case is not treated as an error. Consider explicitly failing when any result has amountPaid > 0 and isValid == false, and separately requiring at least one paid quote to exist.
| for result in &results { | |
| if result.isValid && result.amountPaid > Amount::ZERO { | |
| valid_paid_count += 1; | |
| } | |
| } | |
| let mut has_invalid_paid: bool = false; | |
| for result in &results { | |
| if result.amountPaid > Amount::ZERO { | |
| if result.isValid { | |
| valid_paid_count += 1; | |
| } else { | |
| has_invalid_paid = true; | |
| } | |
| } | |
| } | |
| if has_invalid_paid { | |
| let xorname_hex = hex::encode(xorname); | |
| return Err(Error::Payment(format!( | |
| "Payment verification failed on-chain for {xorname_hex}: \ | |
| found paid quote(s) with invalid verification ({total_quotes} checked)" | |
| ))); | |
| } |
| candidate_prices.sort_unstable(); // ascending | ||
| // Upper median (index 8 of 16) — matches Solidity's median16 (k = 8) | ||
| let median_price = candidate_prices[candidate_prices.len() / 2]; | ||
| let total_amount = median_price * Amount::from(1u64 << payment_info.depth); | ||
| total_amount / Amount::from(u64::from(payment_info.depth)) |
There was a problem hiding this comment.
1u64 << payment_info.depth will panic in Rust if depth >= 64, and depth is coming from on-chain data. To avoid a potential runtime panic/DoS, compute 2^depth using checked shift (returning an error on overflow) or shift an Amount/U256 value instead of a u64.
| let info = evmlib::merkle_payments::OnChainPaymentInfo { | ||
| depth: 2, | ||
| merkle_payment_timestamp: ts, | ||
| paid_node_addresses: vec![ | ||
| // First paid node: valid (matches candidate 0) | ||
| (RewardsAddress::new([0u8; 20]), 0), | ||
| // First paid node: valid (matches candidate 0, amount matches formula) | ||
| // Expected per-node: median(1024) * 2^2 / 2 = 2048 | ||
| (RewardsAddress::new([0u8; 20]), 0, Amount::from(2048u64)), |
There was a problem hiding this comment.
This test constructs evmlib::merkle_payments::OnChainPaymentInfo, but the verifier’s pool_cache is typed as evmlib::merkle_batch_payment::OnChainPaymentInfo (imported as OnChainPaymentInfo). If evmlib moved this type during the refactor, this will be a type mismatch and fail to compile; using the same OnChainPaymentInfo type/path consistently in tests would avoid that risk and reduce confusion.
| assert!( | ||
| on_chain_amount >= u128::try_from(*amount).expect("amount fits u128"), | ||
| "On-chain amount should be >= paid amount" |
There was a problem hiding this comment.
This assertion allows completedPayments[quoteHash].amount to be greater than the amount that was paid. For a mapping intended to store the exact paid amount, this should likely be an equality check so the test actually validates correctness (and catches over/under-payment bugs).
| assert!( | |
| on_chain_amount >= u128::try_from(*amount).expect("amount fits u128"), | |
| "On-chain amount should be >= paid amount" | |
| assert_eq!( | |
| on_chain_amount, | |
| u128::try_from(*amount).expect("amount fits u128"), | |
| "On-chain amount should equal paid amount" |
| assert!( | ||
| results.first().is_some_and(|r| r.isValid), | ||
| "Real payment should be valid" | ||
| real_result.amount > 0, | ||
| "Real payment should have non-zero amount on-chain" | ||
| ); | ||
| println!("✓ Real payment verified (3 atto)"); | ||
|
|
||
| // Check dummy payments | ||
| for (i, result) in results.iter().skip(1).enumerate() { | ||
| println!(" Dummy payment {}: valid={}", i + 1, result.isValid); | ||
| // Check dummy payments (should have 0 amount) | ||
| for (i, (hash, _, _)) in quote_payments.iter().skip(1).enumerate() { | ||
| let result = payment_vault | ||
| .contract | ||
| .completedPayments(*hash) | ||
| .call() | ||
| .await | ||
| .expect("completedPayments lookup failed"); | ||
|
|
||
| println!(" Dummy payment {}: amount={}", i + 1, result.amount); |
There was a problem hiding this comment.
The test only asserts the real payment is non-zero and prints dummy amounts. To actually validate the SingleNode strategy, it should assert the real payment equals the expected amount (e.g., 3x median) and dummy payments are exactly zero; otherwise the test can pass even if amounts are wrong.
…ics) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, verify() checked each node's own on-chain payment amount. Non-median nodes expected 0 and saw 0, so 4/5 nodes couldn't detect underpayment. Now all nodes check the single median quote's on-chain amount against the expected 3× price. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the fixed 32 GiB LMDB map ceiling with dynamic sizing that adapts to the storage partition. Nodes can now use all available disk space for chunks without an artificial cap. - Startup: map_size = current_db_size + available_space − reserve - Resize-on-demand: when MDB_MAP_FULL is hit (e.g. operator added storage), re-query disk space, resize map, retry the write - Disk-space guard: refuse writes when available space drops below a configurable reserve (default 1 GiB), with a 5-second TTL cache to avoid a statvfs syscall on every put - Safety: resize never shrinks below env.info().map_size so existing data is never at risk - New config: storage.disk_reserve_gb (default 1) - Operator override: storage.db_size_gb > 0 still imposes a hard cap Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ── Disk-space guard (cached — at most one syscall per interval) ─ | ||
| self.check_disk_space_cached()?; | ||
|
|
||
| // Fast-path duplicate check (read-only, no write lock needed). | ||
| // This is an optimistic hint — the authoritative check happens inside |
There was a problem hiding this comment.
put() enforces the disk-space reserve before checking whether the chunk already exists. This changes idempotent behavior: attempting to store an already-present chunk can now fail with an "Insufficient disk space" error even though no write occurs. Consider moving the disk-space guard to after the fast-path duplicate check (and/or only enforcing it when try_put returns New), so duplicates remain a harmless no-op and you avoid unnecessary available_space syscalls.
Replace the batch verifyPayment(DataPayment[5]) contract call with a single completedPayments(quoteHash) lookup on just the median quote, verifying it was paid at least 3x its price on-chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion When multiple quotes share the median price, stable sort ordering may differ between client and verifier, causing the verifier to check the wrong quote hash on-chain. Now checks all tied quotes and accepts if any one was paid correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…wards address generation Add `#[allow(clippy::cast_possible_truncation)]` to clarify safe use of cast, as `i` is always < 5.
Summary
QuotingMetricswith a singleprice: Amountfield onPaymentQuoteandMerklePaymentCandidateNode, matching the updated evmlib APIdata_payments_address+merkle_payments_addressinto a singlepayment_vault_address(DevnetEvmInfo, CLI args)(n / 6000)²in wei — removes f64 arithmetic, uses U256 throughoutcompletedPaymentsmapping directly instead of the removedverify_data_paymentbatch callzero_quoting_metrics()helper and allQuotingMetricsconstruction in testsTest plan
cargo testpasses locally against local evmlib🤖 Generated with Claude Code