Skip to content

fix(payment): enforce single-node proof verification#107

Merged
mickvandijke merged 5 commits into
WithAutonomi:mainfrom
mickvandijke:fix/single-node-payment-verification
May 21, 2026
Merged

fix(payment): enforce single-node proof verification#107
mickvandijke merged 5 commits into
WithAutonomi:mainfrom
mickvandijke:fix/single-node-payment-verification

Conversation

@mickvandijke
Copy link
Copy Markdown
Collaborator

@mickvandijke mickvandijke commented May 20, 2026

Description

This PR tightens the final single-node payment verification path used before a new PUT is cached as paid. The verifier still enforces the existing quote count, distinct encoded peer IDs, content-address binding, quote freshness, ML-DSA public-key-to-peer-ID binding, and quote signature checks. After that, it now requires an attached P2PNode, verifies that this node's peer ID is one of the quoted peers, verifies that the local peer's own quote uses the configured local rewards address, and checks the quoted peer set against find_closest_nodes_local_with_self(xorname, CLOSE_GROUP_SIZE). The close-group check is intentionally local and self-inclusive, so it avoids a request-handler network DHT lookup while requiring at least 5 of 7 quoted peers to match and tolerating up to two missing peers for normal routing-table skew.

For the on-chain part, the verifier no longer delegates to the generic SingleNodePayment::verify() amount-only check. It reconstructs the sorted single-node payment, checks the selected median-priced quote first, falls back to other median-price ties if needed, and validates each candidate with PaymentVaultV2.completedPayments(quote.hash()). A candidate only passes if the recorded amount is at least 3 * quote.price and the stored 16-byte recipient prefix matches the quote rewards address. Full 20-byte recipient equality is still not possible from completedPayments without a contract/ABI change or event-based reconstruction.

SemVer: patch

Tests

  • cargo fmt --all -- --check
  • cargo test payment::verifier
  • cargo clippy --all-targets --all-features -- -D warnings

Copilot AI review requested due to automatic review settings May 20, 2026 14:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Tightens single-node payment proof verification in PaymentVerifier by ensuring the proof’s quoted peers match the verifier’s local close-group view and by strengthening the on-chain validation (median-priced selection, recipient-prefix binding, and under/overpayment handling) against PaymentVault.completedPayments.

Changes:

  • Adds local close-group membership enforcement for all quoted peers in single-node proofs.
  • Replaces the previous SingleNodePayment::verify(...) path with explicit on-chain completedPayments(quoteHash) checks that handle median-price ties and validate rewards-address prefix + paid amount.
  • Extends unit tests to cover close-group set validation and completed-payment amount/recipient-prefix checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/payment/verifier.rs Outdated
Comment thread src/payment/verifier.rs
Copilot AI review requested due to automatic review settings May 20, 2026 16:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/payment/verifier.rs
Copilot AI review requested due to automatic review settings May 20, 2026 16:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/payment/verifier.rs:501

  • The CPU-heavy ML-DSA signature verification runs before the single-node close-group check (which also fails fast if no P2PNode is attached). To reduce DoS surface and avoid wasting CPU when the proof will be rejected due to close-group / attachment constraints, consider running validate_single_node_close_group(...) (or at least the p2p_node attachment + local peer/rewards checks) before spawning the blocking signature verification task.
        // Verify quote signatures (CPU-bound, run off async runtime)
        let peer_quotes = payment.peer_quotes.clone();
        tokio::task::spawn_blocking(move || {
            for (encoded_peer_id, quote) in &peer_quotes {
                if !verify_quote_signature(quote) {

Copy link
Copy Markdown
Collaborator

@grumbach grumbach left a comment

Choose a reason for hiding this comment

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

LGTM, approving. This is the right shape — covers the six invariants you listed at the top and binds the on-chain recipient prefix to the paid quote's own rewards address (not this node's), which fixes the model regression my reverted PR introduced.

A few notes, none merge-blocking:

1. Price floor (the one we discussed). Dropping (b) leaves an economic underpricing hole: an attacker who controls ≥5 nodes in a victim's local close-group view can sign 7 quotes at 1 atto, pay 3 atto to one of their own nodes, and store data on the network for ~gas-only. The close-group / 5-of-7 check forces them to own real RT-resident peers but not to price honestly. Agreed the 4× divisor I picked was a guess — but a baseline-pegged floor (e.g. quote.price >= calculate_price(0) = BASELINE) costs us nothing on legit traffic and shuts the 1-atto path. Worth a follow-up at minimum.

2. Replication ingestion path. validate_local_quoted_peer rejects any proof where this node isn't in the quoted set — correct for fresh PUTs. But if a node becomes newly responsible after an RT shift, the original quote set won't include it. Worth tracing handle_fresh_offer / replication ingestion to confirm it short-circuits via the verified-cache (or via a different path) rather than re-running verify_evm_payment, otherwise legitimate re-replication breaks. If you've already checked that, ignore.

3. 5-of-7 close-group threshold. Same shape as the merkle 13/16; the 2-peer skew tolerance is defensible but reduces to "attacker needs ≥5 RT-close real peers" — same Sybil cost class as the #99 residual. A code comment with the calibration rationale (or a pointer to a test you ran) would help future reviewers not re-derive it.

4. Test coverage of validate_single_node_close_group. The pure helper check_single_node_close_group_match is well-tested; the wiring (P2PNode-required, error path) doesn't have a unit test. Minor.

Ship it.

@mickvandijke
Copy link
Copy Markdown
Collaborator Author

mickvandijke commented May 21, 2026

  1. Price floor (the one we discussed). Dropping (b) leaves an economic underpricing hole: an attacker who controls ≥5 nodes in a victim's local close-group view can sign 7 quotes at 1 atto, pay 3 atto to one of their own nodes, and store data on the network for ~gas-only. The close-group / 5-of-7 check forces them to own real RT-resident peers but not to price honestly. Agreed the 4× divisor I picked was a guess — but a baseline-pegged floor (e.g. quote.price >= calculate_price(0) = BASELINE) costs us nothing on legit traffic and shuts the 1-atto path. Worth a follow-up at minimum.

Hey @grumbach , I agree that the price check is a good idea, but I would like the change to be its own PR as a follow-up because we can then also remove the requirement of sending 7 quotes with payments. Instead we can let clients just send the 1 paid quote with the upload. The node will then just verify if the quote issuer is in its current closest group and if the price is within margin of its own calculated price. If we compare the quote to the local node's price calculation, then there is no need for all the other quotes to be included at all.

If we also include closest group data in quote responses, then clients will be able to "smart" pick the best quote that will be accepted by a majority of peers in the close group for a chunk.

@mickvandijke mickvandijke merged commit bece788 into WithAutonomi:main May 21, 2026
20 of 23 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.

3 participants