Skip to content

e2e: cover RFC-27 proof enforcement with the feature flag set - #4243

Draft
elitegreg wants to merge 1 commit into
gm/dzctl-ip-verifierfrom
gm/e2e-ip-proof-enforcement
Draft

e2e: cover RFC-27 proof enforcement with the feature flag set#4243
elitegreg wants to merge 1 commit into
gm/dzctl-ip-verifierfrom
gm/e2e-ip-proof-enforcement

Conversation

@elitegreg

Copy link
Copy Markdown
Contributor

Closes #4205. Part of RFC-27 (rfcs/rfc27-ip-verification.md, tracker #4194).

Stacked on #4232 — based on gm/dzctl-ip-verifier, which is where the e2e devnet verifier and SetIPOwnershipProofFeatureFlag come from. Must not merge before it. The diff shown here is against that branch.

Summary of Changes

  • require-ip-ownership-proof had no e2e coverage at all. It was never set in any test, and devnet.SetIPOwnershipProofFeatureFlag had zero callers. Five tests now run with the flag set.
  • Wildcard access passes get their first e2e coverage of any kind. All 72 access-pass set call sites under e2e/ name a --client-ip, so the case RFC-27 actually exists for was untested. A pass at the 0.0.0.0 PDA — the shape the shred-oracle issues — authorizes its payer for any routable address, which makes the proof the only thing binding client_ip. Covered both ways on one devnet: with a proof the user binds the observed address, without one the create is rejected.
  • The sentinel exemption is covered, because enforcement must not break the oracle path. The manager is the sentinel authority in a local devnet, so a manager-side user create still succeeds under enforcement while a client-paid one does not — the difference being the transaction payer, which is what is_sentinel compares.
  • An address mismatch is asserted client-side, where the guard actually lives: connect binds its proof request to the address it provisions and refuses a proof for any other.
  • One harness addition: ClientSpec.DaemonClientIP overrides the daemon's -client-ip. Needed because doublezero connect --client-ip is deprecated and explicitly ignored — the address comes from the daemon.

Scope: what is not here, and why

Two facts bound what e2e can usefully assert, and they cut several of the issue's bullets:

Most bad-proof cases never reach the chain. The Rust SDK pre-flights version, payer, client_ip, user_type and the signature before it builds a transaction, and the runtime's Ed25519 precompile rejects bad signatures before the program runs. Of the program's proof errors only IpOwnershipProofRequired (105) and IpProofEpochOutOfWindow (110) are reachable end to end. Testing the others here would assert CLI strings while duplicating program tests.

The program already covers them. smartcontract/programs/doublezero-serviceability/tests/user_ip_proof_test.rs has 36 tests spanning every rejection condition, epoch window included, using warp_to_epoch — a solana-program-test facility with no equivalent against a real validator.

So this PR asserts 105 onchain and puts the rest of its weight on integration: a real verifier, a real ledger, a real tunnel, real BGP.

Deferred — stale epoch (issue bullet 5). Feasible, but it needs two harness additions and is better as its own change: LedgerSpec.SlotsPerEpoch plumbed through the ledger entrypoint as --slots-per-epoch 32 (silently ignored if the ledger volume already exists), plus IPVerifierSpec knobs for epoch_refresh_secs/max_epoch_age_secs so the service keeps signing a stale epoch while the ledger moves on. Today the devnet validator uses the default 432,000-slot epoch, so the epoch never advances — every existing proof test lives inside epoch 0, where the {epoch-1, epoch} window degenerates to {0}. Error 110 is the only other reachable onchain rejection, so this is the one remaining case with real e2e value; happy to file a follow-up issue.

Deferred — rotated verifier key (bullet 7). Already covered by TestE2E_IPOwnershipProof_UntrustedSigner in #4232, and it can only ever be a client-side assertion: the SDK's verify catches it before the transaction is built.

Diff Breakdown

Category Files Lines (+/-) Net
Tests 1 +245 / -0 +245
Docs 1 +24 / -0 +24
Core logic 1 +11 / -1 +10
Total 3 +280 / -1 +279

Almost entirely tests; the one harness change is a spec field and a two-line branch.

Key files (click to expand)
  • e2e/ip_ownership_proof_enforcement_test.go — the five enforcement tests
  • e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md — a "testing enforcement" section, including the two traps below
  • e2e/internal/devnet/client.goClientSpec.DaemonClientIP

Testing Verification

All five pass against real devnets, each run individually per the repo's guidance (and the issue's acceptance criterion).

  • EnforcedWithValidProof — proof obtained and attached, ✅ User Provisioned, and WaitForTunnelUp reaches BGP Session Up. Enforcement disturbs nothing downstream of the proof.
  • EnforcedRejectsMissingProof — a client with no verifier is refused. Asserts the specific error, not merely that connect failed.
  • WildcardAccessPass — the one that matters. With a proof the user binds the observed address; without one the program rejects it:
    Program log: Instruction: CreateUser(... client_ip: 9.210.238.101, ... ip_proof: false)
    Program log: IP ownership proof required but none supplied
    Program 7CTniUa88iJKUHTrCkB4TjAoG6TD7AMivhQeuqN2LPtX failed: custom program error: 0x69
    
  • SentinelExemption — manager-side user create with no proof succeeds under enforcement.
  • ClientIpMismatchRefused — daemon set to an address the container does not own, so the verifier observes the real CYOA source:
    ❌  The verification service observed this host at 9.130.200.100, but the daemon is provisioning 9.0.0.7.
    

Also re-ran the three existing TestE2E_IPOwnershipProof tests from #4232, since DaemonClientIP touches shared client startup — all pass. (One run hit an unrelated controller container-start flake from leftover containers; clean after make e2e-test-cleanup.)

Notes for the reviewer

  • Each test takes its own devnet because the feature flag is devnet-global. That is five more devnets in the suite; they are single-device, single-client, so on the light end, but worth knowing given e2e: run an ip-verifier in every devnet and cover the RFC-27 proof outcomes #4232 already adds a verifier container everywhere.
  • Every rejection assertion keys on the specific error string rather than just a non-zero exit. A test that passes because connect broke for an unrelated reason was the main risk in this set.
  • Two traps I hit and wrote into the docs, in case they save someone else the time: the manager is the sentinel authority locally, so "flag on rejects everything" is false for manager-driven creates; and doublezero connect --client-ip is deprecated and ignored, so the daemon flag is the only way to move the provisioned address.

require-ip-ownership-proof was never set in any e2e test, so the enforcement
dimension of RFC-27 had no coverage at all and SetIPOwnershipProofFeatureFlag
had no callers.

Five tests with the flag set. The working path still reaches BGP. A client with
no verifier to reach is refused by the program with IpOwnershipProofRequired,
which is one of only two proof errors reachable end to end: the SDK pre-flights
version, payer, client_ip, user_type and the signature before building a
transaction, so those rejections never leave the client, and the epoch window
needs a ledger epoch a devnet never advances past 0.

The wildcard access pass is the case RFC-27 exists for and had no e2e coverage
of any kind: all 72 access-pass call sites in e2e name a --client-ip. A pass at
the 0.0.0.0 PDA authorizes its payer for any routable address, which is the
shape the shred-oracle issues, so the proof is the only thing binding client_ip.
Covered both ways on one devnet: with a proof the user binds the observed
address, without one the create is rejected.

The sentinel exemption is covered too, because enforcement must not break the
oracle path. The manager is the sentinel authority in a local devnet, so a
manager-side user create still succeeds while a client-paid one does not.

An address mismatch is asserted client-side, where the guard actually lives:
connect binds its proof request to the address it provisions and refuses a proof
for any other. ClientSpec.DaemonClientIP sets the daemon to an address the
container does not own, which is what makes the two disagree.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant