Skip to content

Commit 5c10cc9

Browse files
MoonBoi9001claude
andcommitted
feat(dips): reject agreement proposals from untrusted senders
The payments service recovers each proposal's signer but accepts any valid signature, with no check on who sent it. Require the signer to hold the on-chain agreement-manager role, read from the indexing-payments subgraph, and reject senders that are not in that set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5be1296 commit 5c10cc9

8 files changed

Lines changed: 658 additions & 8 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/config/src/config.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,10 @@ impl MetricsConfig {
473473
#[allow(deprecated)] // Allow using deprecated EscrowSubgraphConfig type
474474
pub struct SubgraphsConfig {
475475
pub network: NetworkSubgraphConfig,
476+
/// Indexing-payments subgraph, read for the DIPs agreement-manager role set.
477+
/// Required when DIPs is enabled.
478+
#[serde(default)]
479+
pub indexing_payments: Option<SubgraphConfig>,
476480
#[deprecated(note = "V2 escrow accounts are in the network subgraph; this field is ignored.")]
477481
#[serde(default)]
478482
pub escrow: Option<EscrowSubgraphConfig>,
@@ -682,6 +686,14 @@ pub struct DipsConfig {
682686
/// RecurringCollector address: the EIP-712 verifying contract used to recover
683687
/// the signer of incoming RCA proposals. Required when DIPs is enabled.
684688
pub recurring_collector: Option<Address>,
689+
/// How often to re-pull the agreement-manager role set from the subgraph.
690+
pub role_refresh_interval_secs: u64,
691+
/// Extra time past the refresh interval that a cached role set stays trusted
692+
/// while refreshes fail, before the gate fails closed (the fail-open window).
693+
pub role_failopen_grace_secs: u64,
694+
/// Max seconds the role subgraph head may lag wall-clock before its data is
695+
/// treated as unreliable. 0 disables the check.
696+
pub role_subgraph_max_lag_secs: u64,
685697
}
686698

687699
impl Default for DipsConfig {
@@ -694,6 +706,9 @@ impl Default for DipsConfig {
694706
min_grt_per_billion_entities_per_30_days: GRT::ZERO,
695707
additional_networks: BTreeMap::new(),
696708
recurring_collector: None,
709+
role_refresh_interval_secs: 86_400,
710+
role_failopen_grace_secs: 3_600,
711+
role_subgraph_max_lag_secs: 1_800,
697712
}
698713
}
699714
}

crates/dips/Cargo.toml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ db = [
2323
"dep:graph-networks-registry",
2424
"dep:serde",
2525
"dep:serde_yaml",
26+
"dep:bytes",
2627
]
2728

2829
[dependencies]
@@ -37,7 +38,7 @@ build-info = { workspace = true, optional = true }
3738
indexer-monitor = { path = "../monitor", optional = true }
3839
thiserror.workspace = true
3940
graph-networks-registry = { workspace = true, optional = true }
40-
serde = { workspace = true, optional = true }
41+
serde = { workspace = true, optional = true, features = ["derive"] }
4142
serde_yaml = { version = "0.9", optional = true }
4243

4344
# IPFS client dependencies
@@ -58,6 +59,9 @@ indexer-monitor = { path = "../monitor" }
5859
graph-networks-registry.workspace = true
5960
build-info.workspace = true
6061
rand = "0.8"
62+
reqwest = { workspace = true, features = ["json"] }
63+
serde_json.workspace = true
64+
wiremock.workspace = true
6165

6266
[build-dependencies]
6367
tonic-build = { workspace = true, optional = true }

crates/dips/src/lib.rs

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ mod registry;
7171
#[cfg(feature = "rpc")]
7272
pub mod server;
7373
pub mod store;
74+
pub mod trusted_signers;
7475

7576
use thiserror::Error;
7677
use uuid::Uuid;
@@ -195,6 +196,10 @@ pub enum DipsError {
195196
DeadlineExpired { deadline: u64, now: u64 },
196197
#[error("agreement end time {ends_at} has already passed (current time: {now})")]
197198
AgreementExpired { ends_at: u64, now: u64 },
199+
#[error("sender {signer} is not authorised to send agreement proposals")]
200+
SenderNotTrusted { signer: Address },
201+
#[error("could not verify sender authorisation: {0}")]
202+
TrustVerificationUnavailable(String),
198203
}
199204

200205
#[cfg(feature = "rpc")]
@@ -304,17 +309,18 @@ pub async fn validate_and_create_rca(
304309
registry,
305310
additional_networks,
306311
rca_domain,
312+
trusted_signers,
307313
} = ctx.as_ref();
308314

309315
// Decode SignedRCA
310316
let signed_rca = SignedRecurringCollectionAgreement::abi_decode(rca_bytes.as_ref())
311317
.map_err(|e| DipsError::AbiDecoding(e.to_string()))?;
312318

313-
// Authenticate the sender before acting on the proposal: recover the EIP-712
314-
// signer. PR B gates this address against the on-chain agreement-manager role
315-
// set; for now any cryptographically valid signature passes.
319+
// Authenticate then authorize the sender: recover the EIP-712 signer, then
320+
// require it to hold the on-chain agreement-manager role before doing any work.
316321
let signer = signed_rca.recover_signer(rca_domain)?;
317322
tracing::debug!(%signer, "recovered RCA signer");
323+
trusted_signers.verify_trusted(signer).await?;
318324

319325
// Validate service provider
320326
signed_rca.validate(expected_service_provider)?;
@@ -465,6 +471,7 @@ mod test {
465471
rca_eip712_domain,
466472
server::DipsServerContext,
467473
store::{FailingRcaStore, InMemoryRcaStore},
474+
trusted_signers::{StaticTrustedSigners, TrustedSignerSource},
468475
AcceptIndexingAgreementMetadata, DipsError, IndexingAgreementTermsV1,
469476
RecurringCollectionAgreement, SignedRecurringCollectionAgreement,
470477
};
@@ -487,6 +494,12 @@ mod test {
487494
rca_eip712_domain(1337, Address::repeat_byte(0xCC))
488495
}
489496

497+
fn trusted_signers_for_test() -> Arc<dyn TrustedSignerSource> {
498+
Arc::new(StaticTrustedSigners(HashSet::from([
499+
test_signer().address()
500+
])))
501+
}
502+
490503
fn create_test_context() -> Arc<DipsServerContext> {
491504
Arc::new(DipsServerContext {
492505
rca_store: Arc::new(InMemoryRcaStore::default()),
@@ -499,9 +512,41 @@ mod test {
499512
registry: Arc::new(crate::registry::test_registry()),
500513
additional_networks: Arc::new(BTreeMap::new()),
501514
rca_domain: test_rca_domain(),
515+
trusted_signers: trusted_signers_for_test(),
502516
})
503517
}
504518

519+
#[tokio::test]
520+
async fn test_validate_and_create_rca_untrusted_signer_rejected() {
521+
let service_provider = Address::repeat_byte(0x11);
522+
let rca = create_test_rca(
523+
Address::repeat_byte(0x42),
524+
service_provider,
525+
U256::from(200),
526+
U256::from(100),
527+
);
528+
529+
// Trusted set excludes the test signer, so a valid signature is rejected.
530+
let ctx = Arc::new(DipsServerContext {
531+
rca_store: Arc::new(InMemoryRcaStore::default()),
532+
ipfs_fetcher: Arc::new(MockIpfsFetcher::default()),
533+
price_calculator: Arc::new(PriceCalculator::new(
534+
HashSet::from(["mainnet".to_string()]),
535+
BTreeMap::from([("mainnet".to_string(), U256::from(100))]),
536+
U256::from(50),
537+
)),
538+
registry: Arc::new(crate::registry::test_registry()),
539+
additional_networks: Arc::new(BTreeMap::new()),
540+
rca_domain: test_rca_domain(),
541+
trusted_signers: Arc::new(StaticTrustedSigners::default()),
542+
});
543+
let rca_bytes = rca_to_wire_bytes(rca);
544+
545+
let result = super::validate_and_create_rca(ctx, &service_provider, rca_bytes).await;
546+
547+
assert!(matches!(result, Err(DipsError::SenderNotTrusted { .. })));
548+
}
549+
505550
/// Sign an RCA with the fixed test key over the test domain and ABI-encode
506551
/// the wrapper, producing the wire bytes `validate_and_create_rca` decodes
507552
/// and recovers the signer from.
@@ -845,6 +890,7 @@ mod test {
845890
registry: Arc::new(crate::registry::test_registry()),
846891
additional_networks: Arc::new(BTreeMap::new()),
847892
rca_domain: test_rca_domain(),
893+
trusted_signers: trusted_signers_for_test(),
848894
});
849895

850896
let rca_bytes = rca_to_wire_bytes(rca);
@@ -1019,6 +1065,7 @@ mod test {
10191065
registry: Arc::new(crate::registry::test_registry()),
10201066
additional_networks: Arc::new(BTreeMap::new()),
10211067
rca_domain: test_rca_domain(),
1068+
trusted_signers: trusted_signers_for_test(),
10221069
});
10231070

10241071
let rca_bytes = rca_to_wire_bytes(rca);
@@ -1054,6 +1101,7 @@ mod test {
10541101
registry: Arc::new(crate::registry::test_registry()),
10551102
additional_networks: Arc::new(BTreeMap::new()),
10561103
rca_domain: test_rca_domain(),
1104+
trusted_signers: trusted_signers_for_test(),
10571105
});
10581106

10591107
let rca_bytes = rca_to_wire_bytes(rca);
@@ -1089,6 +1137,7 @@ mod test {
10891137
registry: Arc::new(crate::registry::test_registry()),
10901138
additional_networks: Arc::new(BTreeMap::new()),
10911139
rca_domain: test_rca_domain(),
1140+
trusted_signers: trusted_signers_for_test(),
10921141
});
10931142

10941143
let rca_bytes = rca_to_wire_bytes(rca);

crates/dips/src/server.rs

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ use crate::{
5858
SubmitAgreementProposalRequest, SubmitAgreementProposalResponse,
5959
},
6060
store::RcaStore,
61+
trusted_signers::TrustedSignerSource,
6162
DipsError,
6263
};
6364

@@ -81,6 +82,8 @@ pub struct DipsServerContext {
8182
pub additional_networks: Arc<BTreeMap<String, String>>,
8283
/// EIP-712 domain for recovering the RCA signer (RecurringCollector).
8384
pub rca_domain: Eip712Domain,
85+
/// Authorises the recovered signer against the on-chain agreement-manager role.
86+
pub trusted_signers: Arc<dyn TrustedSignerSource>,
8487
}
8588

8689
/// DIPS server implementing RCA protocol.
@@ -119,9 +122,12 @@ fn reject_reason_from_error(err: &DipsError) -> RejectReason {
119122
DipsError::AbiDecoding(_)
120123
| DipsError::InvalidSubgraphManifest(_)
121124
| DipsError::InvalidRca(_) => RejectReason::Unspecified,
122-
// A store failure means the proposal was valid but the indexer couldn't persist
123-
// it -- tell dipper this is transient so it retries rather than giving up.
124-
DipsError::UnknownError(_) => RejectReason::IndexerUnavailable,
125+
DipsError::SenderNotTrusted { .. } => RejectReason::SenderNotTrusted,
126+
// A store failure or an unverifiable role set means a valid proposal the
127+
// indexer couldn't act on -- tell dipper it's transient so it retries.
128+
DipsError::TrustVerificationUnavailable(_) | DipsError::UnknownError(_) => {
129+
RejectReason::IndexerUnavailable
130+
}
125131
}
126132
}
127133

@@ -222,8 +228,11 @@ mod tests {
222228
impl DipsServerContext {
223229
pub fn for_testing() -> Arc<Self> {
224230
use std::collections::{BTreeMap, HashSet};
231+
225232
use thegraph_core::alloy::primitives::U256;
226233

234+
use crate::trusted_signers::StaticTrustedSigners;
235+
227236
Arc::new(Self {
228237
rca_store: Arc::new(InMemoryRcaStore::default()),
229238
ipfs_fetcher: Arc::new(MockIpfsFetcher::default()),
@@ -235,6 +244,7 @@ mod tests {
235244
registry: Arc::new(crate::registry::test_registry()),
236245
additional_networks: Arc::new(BTreeMap::new()),
237246
rca_domain: crate::rca_eip712_domain(1337, Address::repeat_byte(0xCC)),
247+
trusted_signers: Arc::new(StaticTrustedSigners::default()),
238248
})
239249
}
240250
}
@@ -472,6 +482,28 @@ mod tests {
472482
assert_eq!(reason, RejectReason::IndexerUnavailable);
473483
}
474484

485+
#[test]
486+
fn test_reject_reason_sender_not_trusted() {
487+
let err = DipsError::SenderNotTrusted {
488+
signer: Address::repeat_byte(0x55),
489+
};
490+
491+
assert_eq!(
492+
super::reject_reason_from_error(&err),
493+
RejectReason::SenderNotTrusted
494+
);
495+
}
496+
497+
#[test]
498+
fn test_reject_reason_trust_unverifiable_is_transient() {
499+
let err = DipsError::TrustVerificationUnavailable("role subgraph down".to_string());
500+
501+
assert_eq!(
502+
super::reject_reason_from_error(&err),
503+
RejectReason::IndexerUnavailable
504+
);
505+
}
506+
475507
#[test]
476508
fn test_reject_detail_sanitizes_store_errors() {
477509
// Arrange: the raw database error mentions schema internals.

0 commit comments

Comments
 (0)