From b39bbef5b67d3e85f2e9400e2a8beb9968b1eb5a Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Tue, 17 Mar 2026 20:01:20 -0500 Subject: [PATCH 01/12] fix: use version 0 for IndexingAgreementVersion.V1 in metadata validation The Solidity enum IndexingAgreementVersion has V1 as its first variant, which encodes as 0 in the ABI. The validation check was comparing against 1, causing all valid V1 proposals to be rejected with UnsupportedMetadataVersion. Test data also updated to use version 0. Companion to edgeandnode/dipper#583. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/dips/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 5fe6f912b..45a84c511 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -410,8 +410,8 @@ pub async fn validate_and_create_rca( )) })?; - // Only support version 1 terms for now - if metadata.version != 1 { + // Only support V1 terms (IndexingAgreementVersion.V1 = 0 in Solidity enum) + if metadata.version != 0 { return Err(DipsError::UnsupportedMetadataVersion(metadata.version)); } @@ -559,7 +559,7 @@ mod test { let metadata = AcceptIndexingAgreementMetadata { // Any bytes32 works - MockIpfsFetcher ignores the deployment ID subgraphDeploymentId: FixedBytes::ZERO, - version: 1, + version: 0, // IndexingAgreementVersion.V1 = 0 terms: terms.abi_encode().into(), }; @@ -814,7 +814,7 @@ mod test { let metadata = AcceptIndexingAgreementMetadata { subgraphDeploymentId: FixedBytes::ZERO, - version: 1, + version: 0, // IndexingAgreementVersion.V1 = 0 terms: terms.abi_encode().into(), }; @@ -858,7 +858,7 @@ mod test { let metadata = AcceptIndexingAgreementMetadata { subgraphDeploymentId: FixedBytes::ZERO, - version: 1, + version: 0, // IndexingAgreementVersion.V1 = 0 terms: terms.abi_encode().into(), }; From e0714580b52139bed6e7407ad3b1185da8656123 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Wed, 4 Mar 2026 11:48:50 -0500 Subject: [PATCH 02/12] feat(dips): log proposal rejections at INFO level When an RCA proposal is rejected, log the rejection reason, error, and deployment ID (when decodable) at INFO level. Previously the rejection was returned via the gRPC response with no server-side log at INFO, making debugging difficult without access to the client. Co-Authored-By: Claude Opus 4.6 --- crates/dips/src/lib.rs | 11 +++++++++++ crates/dips/src/server.rs | 8 +++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 45a84c511..436d6b619 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -350,6 +350,17 @@ fn bytes32_to_ipfs_hash(bytes: &[u8; 32]) -> String { bs58::encode(&multihash).into_string() } +/// Try to extract the deployment ID from raw signed RCA bytes. +/// +/// Best-effort: returns `None` if any decoding step fails. +pub(crate) fn try_extract_deployment_id(rca_bytes: &[u8]) -> Option { + let signed_rca = SignedRecurringCollectionAgreement::abi_decode(rca_bytes).ok()?; + let metadata = + AcceptIndexingAgreementMetadata::abi_decode(signed_rca.agreement.metadata.as_ref()) + .ok()?; + Some(bytes32_to_ipfs_hash(&metadata.subgraphDeploymentId.0)) +} + /// Validate and create a RecurringCollectionAgreement. /// /// Performs validation: diff --git a/crates/dips/src/server.rs b/crates/dips/src/server.rs index caef201c8..84a10682c 100644 --- a/crates/dips/src/server.rs +++ b/crates/dips/src/server.rs @@ -152,6 +152,7 @@ impl IndexerDipsService for DipsServer { // Validate and store RCA let domain = crate::rca_eip712_domain(self.chain_id, self.recurring_collector); + let deployment_id = crate::try_extract_deployment_id(&signed_voucher); match crate::validate_and_create_rca( self.ctx.clone(), &domain, @@ -169,7 +170,12 @@ impl IndexerDipsService for DipsServer { } Err(e) => { let reject_reason = reject_reason_from_error(&e); - tracing::warn!(error = %e, reason = ?reject_reason, "RCA rejected"); + tracing::info!( + error = %e, + reason = ?reject_reason, + deployment_id = deployment_id.as_deref().unwrap_or("unknown"), + "RCA proposal rejected" + ); Ok(Response::new(SubmitAgreementProposalResponse { response: ProposalResponse::Reject.into(), reject_reason: reject_reason.into(), From ae9f3765522d7c72b24d64df3f6c75cd36c08a4e Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Wed, 4 Mar 2026 13:49:20 -0500 Subject: [PATCH 03/12] style: fix nightly fmt Co-Authored-By: Claude Opus 4.6 --- crates/dips/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 436d6b619..2111f3e7f 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -356,8 +356,7 @@ fn bytes32_to_ipfs_hash(bytes: &[u8; 32]) -> String { pub(crate) fn try_extract_deployment_id(rca_bytes: &[u8]) -> Option { let signed_rca = SignedRecurringCollectionAgreement::abi_decode(rca_bytes).ok()?; let metadata = - AcceptIndexingAgreementMetadata::abi_decode(signed_rca.agreement.metadata.as_ref()) - .ok()?; + AcceptIndexingAgreementMetadata::abi_decode(signed_rca.agreement.metadata.as_ref()).ok()?; Some(bytes32_to_ipfs_hash(&metadata.subgraphDeploymentId.0)) } From e6b5824649db35183971959c336dc5c35c21ce00 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Thu, 26 Mar 2026 18:11:21 -0500 Subject: [PATCH 04/12] feat(dips): log startup configuration at INFO level Log supported networks, recurring collector address, IPFS URL, and per-network minimum pricing when DIPs is enabled. Previously only a warning was emitted when supported_networks was empty. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/service/src/service.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/service/src/service.rs b/crates/service/src/service.rs index cdca73525..3b85284c0 100644 --- a/crates/service/src/service.rs +++ b/crates/service/src/service.rs @@ -243,6 +243,26 @@ pub async fn run() -> anyhow::Result<()> { ); } + tracing::info!( + supported_networks = ?supported_networks, + recurring_collector = %recurring_collector, + ipfs_url = %ipfs_url, + "DIPs configuration loaded" + ); + for (network, grt) in &min_grt_per_30_days { + tracing::info!( + network = %network, + min_grt_per_30_days_wei = %grt.wei(), + "DIPs network pricing" + ); + } + if let Some(entity_price) = &min_grt_per_billion_entities_per_30_days { + tracing::info!( + min_grt_per_billion_entities_per_30_days_wei = %entity_price.wei(), + "DIPs entity pricing" + ); + } + let addr: SocketAddr = format!("{host}:{port}") .parse() .with_context(|| format!("Invalid DIPS host:port '{host}:{port}'"))?; From b60ab8737005b4288680e65d337ad0ceb56aa374 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Thu, 26 Mar 2026 18:41:51 -0500 Subject: [PATCH 05/12] fix: correct iterator and type errors in DIPs startup logging min_grt_per_30_days is destructured from a reference, so use .iter() instead of &ref to avoid &&BTreeMap. min_grt_per_billion_entities is GRT not Option, so remove the if-let-Some wrapper. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/service/src/service.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/service/src/service.rs b/crates/service/src/service.rs index 3b85284c0..fec64a2f1 100644 --- a/crates/service/src/service.rs +++ b/crates/service/src/service.rs @@ -249,19 +249,17 @@ pub async fn run() -> anyhow::Result<()> { ipfs_url = %ipfs_url, "DIPs configuration loaded" ); - for (network, grt) in &min_grt_per_30_days { + for (network, grt) in min_grt_per_30_days.iter() { tracing::info!( network = %network, min_grt_per_30_days_wei = %grt.wei(), "DIPs network pricing" ); } - if let Some(entity_price) = &min_grt_per_billion_entities_per_30_days { - tracing::info!( - min_grt_per_billion_entities_per_30_days_wei = %entity_price.wei(), - "DIPs entity pricing" - ); - } + tracing::info!( + min_grt_per_billion_entities_per_30_days_wei = %min_grt_per_billion_entities_per_30_days.wei(), + "DIPs entity pricing" + ); let addr: SocketAddr = format!("{host}:{port}") .parse() From 31b93045905aab6fa5406bb240462207c68d9df0 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Thu, 26 Mar 2026 18:16:21 -0500 Subject: [PATCH 06/12] feat(dips): add chain_id and structured fields to price rejection logs Price rejection logs now include chain_id (CAIP-2 identifier) and use structured tracing fields (offered, minimum) instead of format string interpolation. Makes it easier to filter and query rejection events in production log aggregation. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/dips/src/lib.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 2111f3e7f..2747eada8 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -449,6 +449,13 @@ pub async fn validate_and_create_rca( return Err(DipsError::UnsupportedNetwork(network_name.to_string())); } + // Resolve chain ID for logging context + let chain_id = registry + .get_network_by_id(network_name) + .map(|n| n.caip2_id.to_string()) + .or_else(|| additional_networks.get(network_name).cloned()) + .unwrap_or_else(|| "unknown".to_string()); + // Validate price minimums let offered_tokens_per_second = terms.tokensPerSecond; match price_calculator.get_minimum_price(network_name) { @@ -456,10 +463,11 @@ pub async fn validate_and_create_rca( tracing::info!( agreement_id = %agreement_id, network = %network_name, + chain_id = %chain_id, deployment_id = %deployment_id, - "offered tokens_per_second '{}' is lower than minimum price '{}'", - offered_tokens_per_second, - price + offered = %offered_tokens_per_second, + minimum = %price, + "tokens_per_second below minimum, rejecting proposal" ); return Err(DipsError::TokensPerSecondTooLow { network: network_name.to_string(), @@ -472,9 +480,9 @@ pub async fn validate_and_create_rca( tracing::info!( agreement_id = %agreement_id, network = %network_name, + chain_id = %chain_id, deployment_id = %deployment_id, - "network '{}' is not configured in price calculator", - network_name + "network not configured in price calculator, rejecting proposal" ); return Err(DipsError::UnsupportedNetwork(network_name.to_string())); } @@ -486,10 +494,11 @@ pub async fn validate_and_create_rca( tracing::info!( agreement_id = %agreement_id, network = %network_name, + chain_id = %chain_id, deployment_id = %deployment_id, - "offered tokens_per_entity_per_second '{}' is lower than minimum price '{}'", - offered_entity_price, - price_calculator.entity_price() + offered = %offered_entity_price, + minimum = %price_calculator.entity_price(), + "tokens_per_entity_per_second below minimum, rejecting proposal" ); return Err(DipsError::TokensPerEntityPerSecondTooLow { minimum: price_calculator.entity_price(), From e41c3edd9caf3b68998dbf49e4e09ed760c3a780 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Wed, 1 Apr 2026 20:07:51 +0100 Subject: [PATCH 07/12] test: add shared test vector for derive_agreement_id Pins the expected bytes16 output for a fixed set of RCA inputs. The same test vector exists in dipper (dipper-rpc/src/indexer.rs). If either repo's derivation drifts, the test fails with a message pointing to the counterpart. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/dips/src/lib.rs | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 2747eada8..bd9085232 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -630,6 +630,55 @@ mod test { assert_eq!(id.as_bytes(), &expected_hash[..16]); } + /// Shared test vector with dipper (dipper-rpc/src/indexer.rs). + /// Both repos must produce the same bytes16 for this input. + /// If this test fails, the derivation has drifted from the on-chain + /// contract and/or from dipper -- cancellations and agreement + /// matching will break silently. + #[test] + fn test_derive_agreement_id_shared_vector() { + let rca = RecurringCollectionAgreement { + deadline: 1700000300, + endsAt: 1700086400, + payer: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" + .parse() + .unwrap(), + dataService: "0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9" + .parse() + .unwrap(), + serviceProvider: "0xf4EF6650E48d099a4972ea5B414daB86e1998Bd3" + .parse() + .unwrap(), + maxInitialTokens: U256::from(1_000_000_000_000_000_000u64), + maxOngoingTokensPerSecond: U256::from(1_000_000_000_000_000u64), + minSecondsPerCollection: 3600, + maxSecondsPerCollection: 86400, + nonce: U256::from(0x019d44a86ac97e938672e2501fe630f2u128), + metadata: Default::default(), + }; + + let id = derive_agreement_id(&rca); + + // Pinned expected value. If this fails, check: + // 1. dipper: dipper-rpc/src/indexer.rs test_derive_agreement_id_shared_vector + // 2. Solidity: RecurringCollector._generateAgreementId() + let expected: [u8; 16] = [ + 0x55, 0x79, 0x42, 0xae, 0xfa, 0xb6, 0x16, 0x09, 0xcf, 0xb9, 0xee, 0x14, 0xd3, 0x09, + 0xa1, 0x7e, + ]; + assert_eq!( + id.as_bytes(), + &expected, + "derive_agreement_id output does not match pinned shared vector. \ + Actual: 0x{} -- update this test AND the matching test in \ + dipper (dipper-rpc/src/indexer.rs)", + id.as_bytes() + .iter() + .map(|b| format!("{b:02x}")) + .collect::() + ); + } + #[tokio::test] async fn test_validate_and_create_rca_success() { let payer_signer = PrivateKeySigner::random(); From 67465ea4a7db98ecdea805e82e472783abbf23d5 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Wed, 6 May 2026 22:19:14 +0800 Subject: [PATCH 08/12] feat(dips): add conditions field to indexing agreement struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The on-chain payment-agreement contract gained a new field between maxSecondsPerCollection and nonce — a set of flags that declares optional conditions. The off-chain struct mirrors the on-chain layout for hashing, so it must match exactly. --- crates/dips/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index bd9085232..b9a44efc4 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -120,6 +120,7 @@ sol! { /// Matches `IRecurringCollector.RecurringCollectionAgreement` exactly. /// The agreement ID is derived on-chain via /// `bytes16(keccak256(abi.encode(payer, dataService, serviceProvider, deadline, nonce)))`. + /// Note: `conditions` is NOT included in the agreement ID preimage. #[derive(Debug, PartialEq)] struct RecurringCollectionAgreement { uint64 deadline; @@ -131,6 +132,7 @@ sol! { uint256 maxOngoingTokensPerSecond; uint32 minSecondsPerCollection; uint32 maxSecondsPerCollection; + uint16 conditions; uint256 nonce; bytes metadata; } From f709cf2b6cc05be75b879c4b7936922b6d1396db Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Wed, 6 May 2026 22:20:23 +0800 Subject: [PATCH 09/12] refactor(dips): drop signature check from proposal validation The proposal validator used to recover a signature from each incoming proposal and reject anything not signed by an address registered in on-chain escrow. The smart contract now enforces that authorization at acceptance time, so the check is removed. --- crates/dips/src/lib.rs | 45 +++----- crates/dips/src/server.rs | 38 ++----- crates/dips/src/signers.rs | 200 ---------------------------------- crates/service/src/service.rs | 3 +- 4 files changed, 21 insertions(+), 265 deletions(-) delete mode 100644 crates/dips/src/signers.rs diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index b9a44efc4..26a274c4a 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -74,7 +74,6 @@ pub mod proto; mod registry; #[cfg(feature = "rpc")] pub mod server; -pub mod signers; pub mod store; use thiserror::Error; @@ -216,8 +215,6 @@ pub enum DipsError { }, #[error("tokens per entity per second {offered} is below configured minimum {minimum}")] TokensPerEntityPerSecondTooLow { minimum: U256, offered: U256 }, - #[error("signer {0} not authorised")] - SignerNotAuthorised(Address), #[error("cancelled_by is expected to match the signer")] UnexpectedSigner, // misc @@ -300,37 +297,19 @@ impl RecurringCollectionAgreement { } impl SignedRecurringCollectionAgreement { - /// Validate the RCA signature and basic fields. + /// Validate proposal-time fields. /// - /// Checks: - /// - EIP-712 signature is valid and recovers to an authorized signer for the payer - /// - Signer is authorized for the payer (via escrow accounts) - /// - Service provider matches expected indexer address - pub fn validate( - &self, - signer_validator: &Arc, - domain: &Eip712Domain, - expected_service_provider: &Address, - ) -> Result<(), DipsError> { - let sig = Signature::try_from(self.signature.as_ref()) - .map_err(|err| DipsError::InvalidSignature(err.to_string()))?; - - let payer = self.agreement.payer; - let signer = sig - .recover_address_from_prehash(&self.agreement.eip712_signing_hash(domain)) - .map_err(|err| DipsError::InvalidSignature(err.to_string()))?; - - signer_validator - .validate(&payer, &signer) - .map_err(|_| DipsError::SignerNotAuthorised(signer))?; - + /// Checks that the service provider matches the expected indexer + /// address. On-chain offer existence is NOT checked here — the offer + /// does not exist yet at proposal time. The contract enforces offer + /// existence when the indexer-agent calls `acceptIndexingAgreement`. + pub fn validate(&self, expected_service_provider: &Address) -> Result<(), DipsError> { if !self.agreement.serviceProvider.eq(expected_service_provider) { return Err(DipsError::UnexpectedServiceProvider { expected: *expected_service_provider, actual: self.agreement.serviceProvider, }); } - Ok(()) } @@ -365,14 +344,17 @@ pub(crate) fn try_extract_deployment_id(rca_bytes: &[u8]) -> Option { /// Validate and create a RecurringCollectionAgreement. /// /// Performs validation: -/// - EIP-712 signature verification +/// - Service provider match +/// - Deadline and expiry checks /// - IPFS manifest fetching and network validation /// - Price minimum enforcement /// +/// On-chain offer existence is NOT checked here — the offer doesn't exist +/// yet at proposal time. The contract enforces it at `acceptIndexingAgreement`. +/// /// Returns the agreement ID if successful, stores in database. pub async fn validate_and_create_rca( ctx: Arc, - domain: &Eip712Domain, expected_service_provider: &Address, rca_bytes: Vec, ) -> Result { @@ -380,7 +362,6 @@ pub async fn validate_and_create_rca( rca_store, ipfs_fetcher, price_calculator, - signer_validator, registry, additional_networks, .. @@ -390,8 +371,8 @@ pub async fn validate_and_create_rca( let signed_rca = SignedRecurringCollectionAgreement::abi_decode(rca_bytes.as_ref()) .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; - // Validate signature and basic fields - signed_rca.validate(signer_validator, domain, expected_service_provider)?; + // Validate service provider + signed_rca.validate(expected_service_provider)?; // Validate deadline hasn't passed let now = std::time::SystemTime::now() diff --git a/crates/dips/src/server.rs b/crates/dips/src/server.rs index 84a10682c..514ee989e 100644 --- a/crates/dips/src/server.rs +++ b/crates/dips/src/server.rs @@ -50,7 +50,6 @@ use crate::{ CancelAgreementResponse, ProposalResponse, RejectReason, SubmitAgreementProposalRequest, SubmitAgreementProposalResponse, }, - signers::SignerValidator, store::RcaStore, DipsError, }; @@ -58,7 +57,6 @@ use crate::{ /// Context for DIPS server with all validation dependencies. /// /// Used for RCA validation: -/// - Signature verification /// - IPFS manifest fetching /// - Price minimum enforcement /// - Network registry lookups @@ -70,8 +68,6 @@ pub struct DipsServerContext { pub ipfs_fetcher: Arc, /// Price calculator for validating minimum prices pub price_calculator: Arc, - /// Signature validator for EIP-712 verification - pub signer_validator: Arc, /// Network registry for supported networks pub registry: Arc, /// Additional networks beyond the registry @@ -100,7 +96,6 @@ fn reject_reason_from_error(err: &DipsError) -> RejectReason { match err { DipsError::TokensPerSecondTooLow { .. } | DipsError::TokensPerEntityPerSecondTooLow { .. } => RejectReason::PriceTooLow, - DipsError::SignerNotAuthorised(_) => RejectReason::SignerNotAuthorised, DipsError::DeadlineExpired { .. } => RejectReason::DeadlineExpired, DipsError::AgreementExpired { .. } => RejectReason::AgreementExpired, DipsError::UnsupportedNetwork(_) => RejectReason::UnsupportedNetwork, @@ -117,10 +112,13 @@ impl IndexerDipsService for DipsServer { /// /// Validates: /// - Version 2 only - /// - EIP-712 signature + /// - Service provider match /// - IPFS manifest and network compatibility /// - Price minimums /// + /// On-chain offer existence is NOT checked — the offer doesn't exist yet + /// at proposal time. The contract enforces it at `acceptIndexingAgreement`. + /// /// Returns Accept/Reject based on validation results. async fn submit_agreement_proposal( &self, @@ -151,15 +149,9 @@ impl IndexerDipsService for DipsServer { } // Validate and store RCA - let domain = crate::rca_eip712_domain(self.chain_id, self.recurring_collector); let deployment_id = crate::try_extract_deployment_id(&signed_voucher); - match crate::validate_and_create_rca( - self.ctx.clone(), - &domain, - &self.expected_payee, - signed_voucher, - ) - .await + match crate::validate_and_create_rca(self.ctx.clone(), &self.expected_payee, signed_voucher) + .await { Ok(agreement_id) => { tracing::info!(%agreement_id, "RCA accepted"); @@ -200,10 +192,7 @@ impl IndexerDipsService for DipsServer { #[cfg(test)] mod tests { use super::*; - use crate::{ - ipfs::MockIpfsFetcher, price::PriceCalculator, signers::NoopSignerValidator, - store::InMemoryRcaStore, - }; + use crate::{ipfs::MockIpfsFetcher, price::PriceCalculator, store::InMemoryRcaStore}; impl DipsServerContext { pub fn for_testing() -> Arc { @@ -218,7 +207,6 @@ mod tests { BTreeMap::from([("mainnet".to_string(), U256::from(200))]), U256::from(100), )), - signer_validator: Arc::new(NoopSignerValidator), registry: Arc::new(crate::registry::test_registry()), additional_networks: Arc::new(BTreeMap::new()), }) @@ -380,18 +368,6 @@ mod tests { assert_eq!(reason, RejectReason::Other); } - #[test] - fn test_reject_reason_signer_not_authorised() { - // Arrange - let err = DipsError::SignerNotAuthorised(Address::ZERO); - - // Act - let reason = super::reject_reason_from_error(&err); - - // Assert - assert_eq!(reason, RejectReason::SignerNotAuthorised); - } - #[test] fn test_reject_reason_deadline_expired() { // Arrange diff --git a/crates/dips/src/signers.rs b/crates/dips/src/signers.rs deleted file mode 100644 index f70b283f4..000000000 --- a/crates/dips/src/signers.rs +++ /dev/null @@ -1,200 +0,0 @@ -// Copyright 2023-, Edge & Node, GraphOps, and Semiotic Labs. -// SPDX-License-Identifier: Apache-2.0 - -//! Signer authorization for DIPS agreements. -//! -//! When Dipper sends an RCA proposal, it's signed by a key that may differ from -//! the payer's address. Payers authorize signers via the PaymentsEscrow contract, -//! and this authorization data is indexed by the network subgraph. -//! -//! # How It Works -//! -//! [`EscrowSignerValidator`] wraps an `EscrowAccountsWatcher` that periodically -//! syncs escrow account data from the network subgraph. When validating an RCA: -//! -//! 1. Recover the signer address from the EIP-712 signature -//! 2. Look up authorized signers for the payer address -//! 3. Verify the recovered signer is in the authorized list -//! -//! # Security Considerations -//! -//! The network subgraph may lag behind chain state. This means: -//! - A newly authorized signer might be rejected briefly (UX issue, not security) -//! - A revoked signer might be accepted briefly (security concern) -//! -//! The **thawing period** on escrow withdrawals mitigates the second case. -//! Payers cannot withdraw funds instantly - they must wait through a thawing -//! period that exceeds the maximum expected subgraph lag. This gives indexers -//! time to collect owed fees before funds disappear. - -use anyhow::anyhow; -use thegraph_core::alloy::primitives::Address; - -pub trait SignerValidator: Sync + Send + std::fmt::Debug { - fn validate(&self, payer: &Address, signer: &Address) -> Result<(), anyhow::Error>; -} - -#[cfg(feature = "db")] -mod escrow_validator { - use super::*; - #[cfg(test)] - use indexer_monitor::EscrowAccounts; - use indexer_monitor::EscrowAccountsWatcher; - - #[derive(Debug)] - pub struct EscrowSignerValidator { - watcher: EscrowAccountsWatcher, - } - - impl EscrowSignerValidator { - pub fn new(watcher: EscrowAccountsWatcher) -> Self { - Self { watcher } - } - - #[cfg(test)] - pub fn mock(accounts: EscrowAccounts) -> Self { - let (_tx, rx) = tokio::sync::watch::channel(accounts); - Self::new(rx) - } - } - - impl SignerValidator for EscrowSignerValidator { - fn validate(&self, payer: &Address, signer: &Address) -> Result<(), anyhow::Error> { - let signers = self.watcher.borrow().get_signers_for_sender(payer); - - if !signers.contains(signer) { - return Err(anyhow!("Signer is not a valid signer for the sender")); - } - - Ok(()) - } - } -} - -#[cfg(feature = "db")] -pub use escrow_validator::EscrowSignerValidator; - -#[derive(Debug)] -pub struct NoopSignerValidator; - -impl SignerValidator for NoopSignerValidator { - fn validate(&self, _payer: &Address, _signer: &Address) -> Result<(), anyhow::Error> { - Ok(()) - } -} - -/// Test validator that always rejects signers. -#[derive(Debug)] -pub struct RejectingSignerValidator; - -impl SignerValidator for RejectingSignerValidator { - fn validate(&self, _payer: &Address, _signer: &Address) -> Result<(), anyhow::Error> { - Err(anyhow!("Signer not authorized (test validator)")) - } -} - -#[cfg(test)] -mod test { - use thegraph_core::alloy::primitives::Address; - - use crate::signers::{NoopSignerValidator, RejectingSignerValidator, SignerValidator}; - - #[test] - fn test_noop_validator_always_accepts() { - // Arrange - let validator = NoopSignerValidator; - let payer = Address::ZERO; - let signer = Address::from_slice(&[0xAB; 20]); - - // Act - let result = validator.validate(&payer, &signer); - - // Assert - assert!(result.is_ok(), "NoopSignerValidator should always accept"); - } - - #[test] - fn test_rejecting_validator_always_rejects() { - // Arrange - let validator = RejectingSignerValidator; - let payer = Address::ZERO; - let signer = Address::from_slice(&[0xAB; 20]); - - // Act - let result = validator.validate(&payer, &signer); - - // Assert - assert!( - result.is_err(), - "RejectingSignerValidator should always reject" - ); - } -} - -#[cfg(all(test, feature = "db"))] -mod escrow_tests { - use std::collections::HashMap; - - use indexer_monitor::EscrowAccounts; - use thegraph_core::alloy::primitives::Address; - - use crate::signers::SignerValidator; - - #[tokio::test] - async fn test_escrow_validator_authorized_signer() { - // Arrange - let payer = Address::ZERO; - let authorized_signer = Address::from_slice(&[1u8; 20]); - let (_tx, watcher) = tokio::sync::watch::channel(EscrowAccounts::new( - HashMap::default(), - HashMap::from_iter(vec![(payer, vec![authorized_signer])]), - )); - let validator = super::EscrowSignerValidator::new(watcher); - - // Act & Assert - assert!( - validator.validate(&payer, &authorized_signer).is_ok(), - "Authorized signer should be accepted" - ); - } - - #[tokio::test] - async fn test_escrow_validator_unauthorized_signer() { - // Arrange - let payer = Address::ZERO; - let authorized_signer = Address::from_slice(&[1u8; 20]); - let unauthorized_signer = Address::from_slice(&[2u8; 20]); - let (_tx, watcher) = tokio::sync::watch::channel(EscrowAccounts::new( - HashMap::default(), - HashMap::from_iter(vec![(payer, vec![authorized_signer])]), - )); - let validator = super::EscrowSignerValidator::new(watcher); - - // Act - let result = validator.validate(&payer, &unauthorized_signer); - - // Assert - assert!(result.is_err(), "Unauthorized signer should be rejected"); - } - - #[tokio::test] - async fn test_escrow_validator_payer_not_signer() { - // Arrange - payer authorizes someone else, not themselves - let payer = Address::ZERO; - let other_signer = Address::from_slice(&[1u8; 20]); - let (_tx, watcher) = tokio::sync::watch::channel(EscrowAccounts::new( - HashMap::default(), - HashMap::from_iter(vec![(payer, vec![other_signer])]), - )); - let validator = super::EscrowSignerValidator::new(watcher); - - // Act - let result = validator.validate(&payer, &payer); - - // Assert - assert!( - result.is_err(), - "Payer signing for themselves without authorization should be rejected" - ); - } -} diff --git a/crates/service/src/service.rs b/crates/service/src/service.rs index fec64a2f1..44c3dd5e0 100644 --- a/crates/service/src/service.rs +++ b/crates/service/src/service.rs @@ -16,7 +16,6 @@ use indexer_dips::{ IndexerDipsService, IndexerDipsServiceServer, }, server::{DipsServer, DipsServerContext}, - signers::EscrowSignerValidator, }; use indexer_monitor::{DeploymentDetails, SubgraphClient}; use release::IndexerServiceRelease; @@ -226,6 +225,7 @@ pub async fn run() -> anyhow::Result<()> { min_grt_per_30_days, min_grt_per_billion_entities_per_30_days, additional_networks, + .. } = dips; // Validate required configuration @@ -306,7 +306,6 @@ pub async fn run() -> anyhow::Result<()> { tokens_per_second, tokens_per_entity_per_second, )), - signer_validator: Arc::new(EscrowSignerValidator::new(v2_watcher.clone())), registry, additional_networks: Arc::new(additional_networks.clone()), }); From faa26d49b37c2bb17d1e73c5a185d84e166f996c Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Wed, 6 May 2026 22:21:18 +0800 Subject: [PATCH 10/12] test(dips): rename wire helper and add layout regression test Rename the test helper to describe what it produces (encoded agreement bytes) rather than the case it was written for. Add a roundtrip test that encodes and decodes a non-zero conditions value so a future struct-layout drift is caught immediately. --- crates/dips/src/lib.rs | 246 ++++++++++++++++------------------------- 1 file changed, 94 insertions(+), 152 deletions(-) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 26a274c4a..ac6d6d23b 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -517,36 +517,42 @@ mod test { derive_agreement_id, ipfs::{FailingIpfsFetcher, MockIpfsFetcher}, price::PriceCalculator, - rca_eip712_domain, server::DipsServerContext, - signers::{NoopSignerValidator, RejectingSignerValidator}, store::{FailingRcaStore, InMemoryRcaStore}, AcceptIndexingAgreementMetadata, DipsError, IndexingAgreementTermsV1, - RecurringCollectionAgreement, + RecurringCollectionAgreement, SignedRecurringCollectionAgreement, }; use thegraph_core::alloy::{ primitives::{keccak256, Address, FixedBytes, U256}, - signers::local::PrivateKeySigner, sol_types::SolValue, }; - const CHAIN_ID: u64 = 42161; // Arbitrum One - fn create_test_context() -> Arc { Arc::new(DipsServerContext { rca_store: Arc::new(InMemoryRcaStore::default()), - ipfs_fetcher: Arc::new(MockIpfsFetcher::default()), // Returns "mainnet" + ipfs_fetcher: Arc::new(MockIpfsFetcher::default()), price_calculator: Arc::new(PriceCalculator::new( HashSet::from(["mainnet".to_string()]), BTreeMap::from([("mainnet".to_string(), U256::from(100))]), U256::from(50), )), - signer_validator: Arc::new(NoopSignerValidator), registry: Arc::new(crate::registry::test_registry()), additional_networks: Arc::new(BTreeMap::new()), }) } + /// Helper: encode an RCA as a `SignedRecurringCollectionAgreement` + /// ABI payload. The signature field is no longer consumed by the + /// validator; this just produces the wire bytes that + /// `validate_and_create_rca` expects to decode. + fn rca_to_wire_bytes(rca: RecurringCollectionAgreement) -> Vec { + SignedRecurringCollectionAgreement { + agreement: rca, + signature: Default::default(), + } + .abi_encode() + } + fn create_test_rca( payer: Address, service_provider: Address, @@ -575,6 +581,7 @@ mod test { maxOngoingTokensPerSecond: U256::from(100), minSecondsPerCollection: 60, maxSecondsPerCollection: 3600, + conditions: 0, nonce: U256::from(1), metadata: metadata.abi_encode().into(), } @@ -592,6 +599,7 @@ mod test { maxOngoingTokensPerSecond: U256::from(10), minSecondsPerCollection: 60, maxSecondsPerCollection: 3600, + conditions: 0, nonce: U256::from(42), metadata: Default::default(), }; @@ -636,6 +644,7 @@ mod test { maxOngoingTokensPerSecond: U256::from(1_000_000_000_000_000u64), minSecondsPerCollection: 3600, maxSecondsPerCollection: 86400, + conditions: 0, nonce: U256::from(0x019d44a86ac97e938672e2501fe630f2u128), metadata: Default::default(), }; @@ -662,26 +671,50 @@ mod test { ); } + /// Guards against drift in the sol! struct layout for the audit-branch + /// `conditions` field. If `conditions` were ever moved, renamed, or + /// dropped from the decoder, this round-trip would either fail to + /// decode or return a corrupted value in the field's slot. + #[test] + fn test_rca_conditions_field_roundtrip() { + let payer = Address::repeat_byte(0x42); + let service_provider = Address::repeat_byte(0x11); + + let mut rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); + rca.conditions = 0xABCD; // arbitrary non-zero 16-bit value + + let encoded = rca_to_wire_bytes(rca.clone()); + let decoded = SignedRecurringCollectionAgreement::abi_decode(encoded.as_ref()) + .expect("roundtrip decode failed"); + + assert_eq!( + decoded.agreement.conditions, 0xABCD, + "conditions field did not survive ABI round-trip" + ); + // Cross-check surrounding fields are intact, so a failure of the + // conditions field isn't silently misread from a neighbour's slot. + assert_eq!( + decoded.agreement.maxSecondsPerCollection, + rca.maxSecondsPerCollection + ); + assert_eq!(decoded.agreement.nonce, rca.nonce); + } + #[tokio::test] async fn test_validate_and_create_rca_success() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); let agreement_id = derive_agreement_id(&rca); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); + let rca_bytes = rca_to_wire_bytes(rca); + let result = - super::validate_and_create_rca(ctx.clone(), &domain, &service_provider, rca_bytes) - .await; + super::validate_and_create_rca(ctx.clone(), &service_provider, rca_bytes).await; - assert!(result.is_ok()); + assert!(result.is_ok(), "got: {:?}", result); assert_eq!(result.unwrap(), agreement_id); // Verify it was stored @@ -694,11 +727,9 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_wrong_service_provider() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); let wrong_service_provider = Address::repeat_byte(0x99); - let recurring_collector = Address::repeat_byte(0x22); let rca = create_test_rca( payer, @@ -707,13 +738,10 @@ mod test { U256::from(100), ); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!( result, @@ -723,21 +751,16 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_tokens_per_second_too_low() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); // Offer 50, minimum is 100 let rca = create_test_rca(payer, service_provider, U256::from(50), U256::from(100)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!( result, @@ -747,21 +770,16 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_entity_price_too_low() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); // Offer 200 tokens/sec (ok), but only 10 entity price (minimum is 50) let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(10)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!( result, @@ -771,17 +789,11 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_unsupported_network() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - // Create context with IPFS fetcher returning unsupported network let ctx = Arc::new(DipsServerContext { rca_store: Arc::new(InMemoryRcaStore::default()), @@ -793,23 +805,20 @@ mod test { BTreeMap::from([("mainnet".to_string(), U256::from(100))]), U256::from(50), )), - signer_validator: Arc::new(NoopSignerValidator), registry: Arc::new(crate::registry::test_registry()), additional_networks: Arc::new(BTreeMap::new()), }); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!(result, Err(DipsError::UnsupportedNetwork(_)))); } #[tokio::test] async fn test_validate_and_create_rca_invalid_metadata_version() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let terms = IndexingAgreementTermsV1 { tokensPerSecond: U256::from(200), @@ -833,17 +842,15 @@ mod test { maxOngoingTokensPerSecond: U256::from(100), minSecondsPerCollection: 60, maxSecondsPerCollection: 3600, + conditions: 0, nonce: U256::from(1), metadata: metadata.abi_encode().into(), }; - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!( result, @@ -853,10 +860,8 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_deadline_expired() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let terms = IndexingAgreementTermsV1 { tokensPerSecond: U256::from(200), @@ -880,27 +885,23 @@ mod test { maxOngoingTokensPerSecond: U256::from(100), minSecondsPerCollection: 60, maxSecondsPerCollection: 3600, + conditions: 0, nonce: U256::from(1), metadata: metadata.abi_encode().into(), }; - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!(result, Err(DipsError::DeadlineExpired { .. }))); } #[tokio::test] async fn test_validate_and_create_rca_agreement_expired() { - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let terms = IndexingAgreementTermsV1 { tokensPerSecond: U256::from(200), @@ -924,17 +925,15 @@ mod test { maxOngoingTokensPerSecond: U256::from(100), minSecondsPerCollection: 60, maxSecondsPerCollection: 3600, + conditions: 0, nonce: U256::from(1), metadata: metadata.abi_encode().into(), }; - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - let ctx = create_test_context(); - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let rca_bytes = rca_to_wire_bytes(rca); + + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; assert!(matches!(result, Err(DipsError::AgreementExpired { .. }))); } @@ -947,15 +946,12 @@ mod test { async fn test_validate_and_create_rca_malformed_abi() { // Arrange let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); let ctx = create_test_context(); let malformed_bytes = vec![0xDE, 0xAD, 0xBE, 0xEF]; // Not valid ABI // Act - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, malformed_bytes).await; + let result = super::validate_and_create_rca(ctx, &service_provider, malformed_bytes).await; // Assert assert!( @@ -965,57 +961,13 @@ mod test { ); } - #[tokio::test] - async fn test_validate_and_create_rca_unauthorized_signer() { - // Arrange - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); - let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); - - let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); - - // Context with rejecting signer validator - let ctx = Arc::new(DipsServerContext { - rca_store: Arc::new(InMemoryRcaStore::default()), - ipfs_fetcher: Arc::new(MockIpfsFetcher::default()), - price_calculator: Arc::new(PriceCalculator::new( - HashSet::from(["mainnet".to_string()]), - BTreeMap::from([("mainnet".to_string(), U256::from(100))]), - U256::from(50), - )), - signer_validator: Arc::new(RejectingSignerValidator), - registry: Arc::new(crate::registry::test_registry()), - additional_networks: Arc::new(BTreeMap::new()), - }); - - // Act - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; - - // Assert - assert!( - matches!(result, Err(DipsError::SignerNotAuthorised(_))), - "Expected SignerNotAuthorised error, got: {:?}", - result - ); - } - #[tokio::test] async fn test_validate_and_create_rca_ipfs_failure() { // Arrange - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); // Context with failing IPFS fetcher let ctx = Arc::new(DipsServerContext { @@ -1026,14 +978,14 @@ mod test { BTreeMap::from([("mainnet".to_string(), U256::from(100))]), U256::from(50), )), - signer_validator: Arc::new(NoopSignerValidator), registry: Arc::new(crate::registry::test_registry()), additional_networks: Arc::new(BTreeMap::new()), }); + let rca_bytes = rca_to_wire_bytes(rca); + // Act - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; // Assert assert!( @@ -1046,15 +998,10 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_manifest_no_network() { // Arrange - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); // Context with IPFS fetcher returning manifest without network let ctx = Arc::new(DipsServerContext { @@ -1065,14 +1012,14 @@ mod test { BTreeMap::from([("mainnet".to_string(), U256::from(100))]), U256::from(50), )), - signer_validator: Arc::new(NoopSignerValidator), registry: Arc::new(crate::registry::test_registry()), additional_networks: Arc::new(BTreeMap::new()), }); + let rca_bytes = rca_to_wire_bytes(rca); + // Act - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; // Assert assert!( @@ -1085,15 +1032,10 @@ mod test { #[tokio::test] async fn test_validate_and_create_rca_store_failure() { // Arrange - let payer_signer = PrivateKeySigner::random(); - let payer = payer_signer.address(); + let payer = Address::repeat_byte(0x42); let service_provider = Address::repeat_byte(0x11); - let recurring_collector = Address::repeat_byte(0x22); let rca = create_test_rca(payer, service_provider, U256::from(200), U256::from(100)); - let domain = rca_eip712_domain(CHAIN_ID, recurring_collector); - let signed_rca = rca.sign(&domain, payer_signer).unwrap(); - let rca_bytes = signed_rca.abi_encode(); // Context with failing store let ctx = Arc::new(DipsServerContext { @@ -1104,14 +1046,14 @@ mod test { BTreeMap::from([("mainnet".to_string(), U256::from(100))]), U256::from(50), )), - signer_validator: Arc::new(NoopSignerValidator), registry: Arc::new(crate::registry::test_registry()), additional_networks: Arc::new(BTreeMap::new()), }); + let rca_bytes = rca_to_wire_bytes(rca); + // Act - let result = - super::validate_and_create_rca(ctx, &domain, &service_provider, rca_bytes).await; + let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await; // Assert assert!( From 5a5adbad1309460a5fc0bb617459548bbca93a1f Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Mon, 11 May 2026 15:03:25 +0800 Subject: [PATCH 11/12] chore(dips): drop dead signature/cancellation code after auth switch Commit f709cf2 removed signature verification and signer-authorization from the validation flow. The supporting types stayed behind: the SignedCancellationRequest / CancellationRequest structs and their sign/validate impls, the rca_eip712_domain and dips_cancellation_eip712_domain helpers, and DipsServer's chain_id and recurring_collector fields. Drop them along with the DipsConfig field, its startup validation, and the matching maximal-config example entry. Also align the module-level docs in dips/lib.rs and dips/server.rs with the validation steps that actually run today. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/config/maximal-config-example.toml | 3 +- crates/config/src/config.rs | 59 +----------- crates/dips/src/lib.rs | 110 +++------------------- crates/dips/src/server.rs | 21 ++--- crates/service/src/service.rs | 17 +--- 5 files changed, 25 insertions(+), 185 deletions(-) diff --git a/crates/config/maximal-config-example.toml b/crates/config/maximal-config-example.toml index e0f417adb..d2fa44580 100644 --- a/crates/config/maximal-config-example.toml +++ b/crates/config/maximal-config-example.toml @@ -193,9 +193,8 @@ max_receipts_per_request = 10000 [dips] host = "0.0.0.0" port = "7601" -recurring_collector = "0x4444444444444444444444444444444444444444" -# Networks you explicitly support indexing. +# Networks you explicitly support indexing. # Proposals from the dipper for you to index networks that are not in the list below are rejected. # See https://github.com/graphprotocol/networks-registry/blob/main/docs/networks-table.md # e.g. supported_networks = ["mainnet", "arbitrum-one"] diff --git a/crates/config/src/config.rs b/crates/config/src/config.rs index f313d3db0..dfeb1b99d 100644 --- a/crates/config/src/config.rs +++ b/crates/config/src/config.rs @@ -639,7 +639,6 @@ fn default_allocation_reconciliation_interval_secs() -> Duration { pub struct DipsConfig { pub host: String, pub port: String, - pub recurring_collector: Address, /// Networks this indexer explicitly supports. Proposals for other networks are rejected. pub supported_networks: HashSet, /// Minimum acceptable GRT per 30 days, per network. Converted to wei/second internally. @@ -654,7 +653,6 @@ impl Default for DipsConfig { DipsConfig { host: "0.0.0.0".to_string(), port: "7601".to_string(), - recurring_collector: Address::ZERO, supported_networks: HashSet::new(), min_grt_per_30_days: BTreeMap::new(), min_grt_per_billion_entities_per_30_days: GRT::ZERO, @@ -776,9 +774,6 @@ mod tests { max_config.tap.trusted_senders = HashSet::from([address!("deadbeefcafebabedeadbeefcafebabedeadbeef")]); max_config.dips = Some(crate::DipsConfig { - recurring_collector: Address( - FixedBytes::<20>::from_str("0x4444444444444444444444444444444444444444").unwrap(), - ), min_grt_per_billion_entities_per_30_days: crate::GRT::from_grt("200"), ..Default::default() }); @@ -1300,23 +1295,6 @@ mod tests { ); } - /// Test that DipsConfig defaults have recurring_collector as Address::ZERO. - /// This is important because the service startup validation checks for this - /// and fails with a clear error message if DIPS is enabled but recurring_collector - /// is not configured. - #[test] - fn test_dips_config_defaults_recurring_collector_zero() { - // Arrange & Act - let dips_config = crate::DipsConfig::default(); - - // Assert - assert_eq!( - dips_config.recurring_collector, - Address::ZERO, - "Default recurring_collector should be Address::ZERO to trigger startup validation" - ); - } - /// Test that DipsConfig defaults have empty supported_networks. /// This triggers a warning at startup that all proposals will be rejected. #[test] @@ -1335,35 +1313,6 @@ mod tests { ); } - /// Test that a DIPS config with only recurring_collector set uses defaults for other fields. - #[test] - fn test_dips_partial_config_uses_defaults() { - // Arrange - create a DipsConfig with just recurring_collector set - let dips_config = crate::DipsConfig { - recurring_collector: Address( - FixedBytes::<20>::from_str("0x1234567890123456789012345678901234567890").unwrap(), - ), - ..Default::default() - }; - - // Assert - recurring_collector is set, others use defaults - assert_ne!( - dips_config.recurring_collector, - Address::ZERO, - "recurring_collector should be set" - ); - assert_eq!(dips_config.host, "0.0.0.0", "host should use default"); - assert_eq!(dips_config.port, "7601", "port should use default"); - assert!( - dips_config.supported_networks.is_empty(), - "supported_networks should default to empty" - ); - assert!( - dips_config.min_grt_per_30_days.is_empty(), - "min_grt_per_30_days should default to empty" - ); - } - /// Test that maximal config with DIPS section parses correctly. #[test] fn test_dips_maximal_config_parses() { @@ -1377,10 +1326,10 @@ mod tests { // Assert let dips = config.dips.expect("maximal config should have DIPS"); - assert_ne!( - dips.recurring_collector, - Address::ZERO, - "recurring_collector should be set in maximal config" + assert_eq!( + dips.min_grt_per_billion_entities_per_30_days, + crate::GRT::from_grt("200"), + "min_grt_per_billion_entities_per_30_days should be set in maximal config" ); } } diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index ac6d6d23b..cdb4fed35 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -28,21 +28,16 @@ //! # Validation Flow //! //! When an RCA arrives, this crate validates: -//! 1. **Signature** - EIP-712 signature recovers to an authorized signer -//! 2. **Signer authorization** - Signer is authorized for the payer (via escrow accounts) -//! 3. **Service provider** - RCA is addressed to this indexer -//! 4. **Timestamps** - Deadline and end time haven't passed -//! 5. **IPFS manifest** - Subgraph deployment exists and is parseable -//! 6. **Network** - Subgraph's network is supported by this indexer -//! 7. **Pricing** - Offered price meets indexer's minimum +//! 1. **Service provider** - RCA is addressed to this indexer +//! 2. **Timestamps** - Deadline and end time haven't passed +//! 3. **IPFS manifest** - Subgraph deployment exists and is parseable +//! 4. **Network** - Subgraph's network is supported by this indexer +//! 5. **Pricing** - Offered price meets indexer's minimum //! -//! # Trust Model -//! -//! Payers deposit funds into the PaymentsEscrow contract and authorize signers. -//! The escrow has a **thawing period** for withdrawals, giving indexers time to -//! collect owed fees before funds can be withdrawn. This crate checks signer -//! authorization via the network subgraph, which may lag chain state slightly. -//! The thawing period protects against this lag. +//! Signature and signer-authorization checks are NOT performed here. With the +//! switch to offer-based authorization, the on-chain `acceptIndexingAgreement` +//! call verifies the signer (via either an ECDSA signature or a pre-stored +//! payer offer) when the indexer-agent submits the acceptance transaction. //! //! # Modules //! @@ -53,15 +48,15 @@ //! - [`ipfs`] - IPFS client for subgraph manifests //! - [`price`] - Minimum price enforcement -use std::{str::FromStr, sync::Arc}; +use std::sync::Arc; use server::DipsServerContext; use thegraph_core::alloy::{ core::primitives::Address, - primitives::{b256, keccak256, ruint::aliases::U256, ChainId, Signature, Uint, B256}, + primitives::{keccak256, ruint::aliases::U256, Uint}, signers::SignerSync, sol, - sol_types::{eip712_domain, Eip712Domain, SolStruct, SolValue}, + sol_types::{Eip712Domain, SolValue}, }; #[cfg(feature = "db")] @@ -82,35 +77,6 @@ use uuid::Uuid; /// Protocol version (seconds-based RCA) pub const PROTOCOL_VERSION: u64 = 2; -/// Create an EIP-712 domain for RecurringCollectionAgreement. -/// -/// Used to sign `RecurringCollectionAgreement` messages. The `verifying_contract` -/// is the deployed RecurringCollector address. -pub fn rca_eip712_domain(chain_id: ChainId, recurring_collector: Address) -> Eip712Domain { - eip712_domain! { - name: "RecurringCollector", - version: "1", - chain_id: chain_id, - verifying_contract: recurring_collector, - } -} - -/// EIP-712 domain salt for DIPs-specific messages. -const EIP712_DOMAIN_SALT: B256 = - b256!("a070ffb1cd7af433c73e0d016c7c4ce31dc1ec7366a3f5d20cfa22a80391e549"); - -/// Create an EIP-712 domain for cancellation requests. -/// -/// Used for signing `CancellationRequest` messages. -pub fn dips_cancellation_eip712_domain(chain_id: ChainId) -> Eip712Domain { - eip712_domain! { - name: "Graph Protocol Indexing Agreement Cancellation", - version: "0", - chain_id: chain_id, - salt: EIP712_DOMAIN_SALT, - } -} - sol! { // === RCA Types (seconds-based RecurringCollectionAgreement) === @@ -159,18 +125,6 @@ sol! { uint256 tokensPerEntityPerSecond; } - // === Cancellation === - - #[derive(Debug, PartialEq)] - struct SignedCancellationRequest { - CancellationRequest request; - bytes signature; - } - - #[derive(Debug, PartialEq)] - struct CancellationRequest { - bytes16 agreement_id; - } } /// Derive the agreement ID deterministically from the RCA fields. @@ -239,46 +193,6 @@ impl From for tonic::Status { } } -impl CancellationRequest { - pub fn sign( - &self, - domain: &Eip712Domain, - signer: S, - ) -> anyhow::Result { - let voucher = SignedCancellationRequest { - request: self.clone(), - signature: signer.sign_typed_data_sync(self, domain)?.as_bytes().into(), - }; - - Ok(voucher) - } -} - -impl SignedCancellationRequest { - // TODO: Validate all values - pub fn validate( - &self, - domain: &Eip712Domain, - expected_signer: &Address, - ) -> Result<(), DipsError> { - let sig = Signature::from_str(&self.signature.to_string()) - .map_err(|err| DipsError::InvalidSignature(err.to_string()))?; - - let signer = sig - .recover_address_from_prehash(&self.request.eip712_signing_hash(domain)) - .map_err(|err| DipsError::InvalidSignature(err.to_string()))?; - - if signer.ne(expected_signer) { - return Err(DipsError::UnexpectedSigner); - } - - Ok(()) - } - pub fn encode_vec(&self) -> Vec { - self.abi_encode() - } -} - // === RCA Implementations === impl RecurringCollectionAgreement { diff --git a/crates/dips/src/server.rs b/crates/dips/src/server.rs index 514ee989e..b3aa1e6c7 100644 --- a/crates/dips/src/server.rs +++ b/crates/dips/src/server.rs @@ -13,8 +13,7 @@ //! │ //! ├─ Version check (must be 2) //! ├─ Size validation (non-empty, max 10KB) -//! ├─ Signature verification -//! ├─ Signer authorization check +//! ├─ Service-provider match //! ├─ Timestamp validation (deadline, endsAt) //! ├─ IPFS manifest fetch //! ├─ Network validation @@ -25,6 +24,11 @@ //! └─> Return Accept/Reject //! ``` //! +//! Signature and signer-authorization checks are not performed here. The +//! on-chain `acceptIndexingAgreement` call verifies the signer (via either +//! an ECDSA signature or a pre-stored payer offer) when the indexer-agent +//! submits the acceptance transaction. +//! //! # Response Behavior //! //! Returns `Accept` if the RCA passes all validation and is stored successfully. @@ -77,7 +81,7 @@ pub struct DipsServerContext { /// DIPS server implementing RCA protocol. /// /// Validates RecurringCollectionAgreement proposals before storage: -/// - EIP-712 signature verification +/// - Service-provider match /// - IPFS manifest fetching and network validation /// - Price minimum enforcement /// @@ -86,9 +90,6 @@ pub struct DipsServerContext { pub struct DipsServer { pub ctx: Arc, pub expected_payee: Address, - pub chain_id: u64, - /// RecurringCollector contract address for EIP-712 domain - pub recurring_collector: Address, } /// Map a DipsError to the appropriate RejectReason for the gRPC response. @@ -220,8 +221,6 @@ mod tests { let server = DipsServer { ctx, expected_payee: Address::ZERO, - chain_id: 1, - recurring_collector: Address::ZERO, }; let request = Request::new(SubmitAgreementProposalRequest { version: 2, @@ -243,8 +242,6 @@ mod tests { let server = DipsServer { ctx, expected_payee: Address::ZERO, - chain_id: 1, - recurring_collector: Address::ZERO, }; let large_payload = vec![0u8; 10_001]; let request = Request::new(SubmitAgreementProposalRequest { @@ -267,8 +264,6 @@ mod tests { let server = DipsServer { ctx, expected_payee: Address::ZERO, - chain_id: 1, - recurring_collector: Address::ZERO, }; let request = Request::new(SubmitAgreementProposalRequest { version: 1, @@ -291,8 +286,6 @@ mod tests { let server = DipsServer { ctx, expected_payee: Address::ZERO, - chain_id: 1, - recurring_collector: Address::ZERO, }; let request = Request::new(CancelAgreementRequest { version: 2, diff --git a/crates/service/src/service.rs b/crates/service/src/service.rs index 44c3dd5e0..d990c1a76 100644 --- a/crates/service/src/service.rs +++ b/crates/service/src/service.rs @@ -21,7 +21,7 @@ use indexer_monitor::{DeploymentDetails, SubgraphClient}; use release::IndexerServiceRelease; use reqwest::Url; use tap_core::tap_eip712_domain; -use thegraph_core::alloy::primitives::{Address, U256}; +use thegraph_core::alloy::primitives::U256; use tokio::{net::TcpListener, signal}; use tokio_util::sync::CancellationToken; use tower_http::normalize_path::NormalizePath; @@ -117,8 +117,6 @@ pub async fn run() -> anyhow::Result<()> { config.blockchain.horizon_receipts_verifier_address(), tap_core::TapVersion::V2, ); - let chain_id = config.blockchain.chain_id as u64; - let host_and_port = config.service.host_and_port; let indexer_address = config.indexer.indexer_address; let ipfs_url = config.service.ipfs_url.clone(); @@ -220,7 +218,6 @@ pub async fn run() -> anyhow::Result<()> { let DipsConfig { host, port, - recurring_collector, supported_networks, min_grt_per_30_days, min_grt_per_billion_entities_per_30_days, @@ -228,14 +225,6 @@ pub async fn run() -> anyhow::Result<()> { .. } = dips; - // Validate required configuration - if *recurring_collector == Address::ZERO { - anyhow::bail!( - "DIPS is enabled but dips.recurring_collector is not configured. \ - Set it to the deployed RecurringCollector contract address." - ); - } - if supported_networks.is_empty() { tracing::warn!( "DIPS enabled but no networks in dips.supported_networks. \ @@ -245,7 +234,6 @@ pub async fn run() -> anyhow::Result<()> { tracing::info!( supported_networks = ?supported_networks, - recurring_collector = %recurring_collector, ipfs_url = %ipfs_url, "DIPs configuration loaded" ); @@ -314,13 +302,10 @@ pub async fn run() -> anyhow::Result<()> { let server = DipsServer { ctx, expected_payee: indexer_address, - chain_id, - recurring_collector: *recurring_collector, }; info!( address = %addr, - recurring_collector = ?recurring_collector, "Starting DIPS gRPC server (RecurringCollectionAgreement validation)" ); From b0ffb876e2a0b75747c6409ef339bd5a378b6dee Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Tue, 12 May 2026 18:21:35 +0800 Subject: [PATCH 12/12] fix(config): drop unused FixedBytes import from test module The test that used FixedBytes was removed earlier when the dead signature/cancellation code was cleaned up after the offer-authorization switch. The import survived and now fails clippy with -D warnings on --all-targets. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/config/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/config/src/config.rs b/crates/config/src/config.rs index dfeb1b99d..ac8f701e6 100644 --- a/crates/config/src/config.rs +++ b/crates/config/src/config.rs @@ -748,7 +748,7 @@ mod tests { use bip39::Mnemonic; use figment::value::Uncased; use sealed_test::prelude::*; - use thegraph_core::alloy::primitives::{address, Address, FixedBytes}; + use thegraph_core::alloy::primitives::{address, Address}; use tracing_test::traced_test; use super::{DatabaseConfig, IndexerConfig, SHARED_PREFIX};