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

[elitegreg]

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

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. One test now runs with the flag set, as four subtests over a single devnet.
• 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: with a proof the user binds the observed address and reaches BGP, without one the create is rejected onchain.
• 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, validated in ClientSpec.Validate alongside the other address-shaped fields. 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.

Not covered — a valid proof on a specific-IP pass. The flag is read at exactly one site, inside the proof == None arm (ip_proof.rs). A supplied valid proof takes a byte-identical program path with the flag set or clear, so such a subtest would duplicate TestE2E_IPOwnershipProof_ValidProof for the price of a client and a 90s WaitForTunnelUp. The "enforcement disturbs nothing downstream" assertion lives on wildcard_pass_with_a_proof instead, which is not a duplicate of anything.

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 from #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	+221 / -0	+221
Docs	2	+27 / -2	+25
Core logic	1	+19 / -1	+18
Total	4	+267 / -3	+264

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

Key files (click to expand)

• e2e/ip_ownership_proof_enforcement_test.go — the enforcement test and its four subtests
• e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md — a "testing enforcement" section, including the two traps below
• e2e/internal/devnet/client.go — ClientSpec.DaemonClientIP and its validation

Testing Verification

TestE2E_IPOwnershipProof_Enforced passes in CI (e2e shard 3, 142.52s) and locally.

• wildcard_pass_with_a_proof — the one that matters. The pass names no address, so the proof is the only thing that could bind client_ip: IP ownership verified for <addr>, ✅ User Provisioned, the user appears in user list at the observed address, and WaitForTunnelUp reaches BGP Session Up. Enforcement disturbs nothing downstream of the proof.
• wildcard_pass_without_a_proof — the same wildcard pass does not rescue a client with no verifier to reach. 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
  

• sentinel_authority_is_exempt — manager-side user create with no proof succeeds under enforcement.
• proof_for_a_different_address_is_refused — 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 TestE2E_IPOwnershipProof_ValidProof locally, since DaemonClientIP touches shared client startup — passes (173s).

Notes for the reviewer

• One devnet for the whole file. The two things a client cannot change after it starts — whether it has a verifier to reach, and what address its daemon provisions — are per-ClientSpec, so each costs one small container rather than a second ledger + manager + controller + cEOS device.
• 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.

[Copilot]

Pull request overview

This pull request adds end-to-end coverage for RFC-27 IP ownership proof enforcement when the require-ip-ownership-proof feature flag is enabled, using real devnets and the real verifier path.

Changes:

• Add a new e2e test suite that enables the enforcement feature flag and asserts the expected allow and deny outcomes, including wildcard access pass behavior and the sentinel exemption.
• Extend the e2e devnet client harness to support overriding the daemon provisioned client IP for mismatch testing.
• Document enforcement testing pitfalls for local devnets and record the change in the changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
e2e/ip_ownership_proof_enforcement_test.go	Adds enforcement-on e2e coverage for RFC-27, including wildcard pass, missing-proof rejection, sentinel exemption, and client IP mismatch refusal.
e2e/internal/devnet/client.go	Adds ClientSpec.DaemonClientIP and wires it into daemon startup arguments.
e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md	Adds guidance for testing enforcement and common pitfalls in local devnets.
CHANGELOG.md	Notes the new e2e coverage for RFC-27 enforcement.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[juan-malbeclabs]

Reviewed the full diff and traced each behavioral claim back through the code it depends on.

Things I verified as correct, so they don't come up below: the wildcard 0.0.0.0 PDA path is real and is preferred over the exact-IP PDA (smartcontract/sdk/rs/src/commands/accesspass/get.rs:27, create_core.rs:207-231), so both wildcard subtests exercise what they claim; access-pass set with no --client-ip does land at Ipv4Addr::UNSPECIFIED; IpOwnershipProofRequired is custom error 105 and its Display string matches the asserted substring; the manager really is sentinel_authority_pk and is_sentinel compares the tx payer; feature-flags set is a read-modify-write of the mask only, so it can't clobber ip_verifier_authority_pk; the mismatch mechanism holds because probe_source_binding returns NotLocal for an unassigned address and falls back to default egress; the DaemonClientIP override touches only the daemon flag, leaving Client.CYOANetworkIP from docker inspect intact; and both new TestE2E_ names are picked up by the shard matrix.

The findings are all about cost and comment accuracy rather than broken tests — the two big ones are that the second devnet doesn't look necessary, and that two of the three flag-set assertions don't actually depend on the flag.

[elitegreg]

Pushed as an amend to the single commit on the branch. All six threads addressed; replies are on each one.

The structural changes, since they are easier to see together than thread by thread:

• One devnet, not two. The mismatch test folded into TestE2E_IPOwnershipProof_Enforced as a fourth subtest on a third client. A separate ledger + manager + controller + cEOS device for one client-side guard was not justified.
• specific_pass_with_a_proof dropped as a duplicate of TestE2E_IPOwnershipProof_ValidProof. Its one non-duplicate assertion, that enforcement disturbs nothing downstream of the proof, moved to wildcard_pass_with_a_proof, so BGP is still asserted under enforcement — one fewer client and one fewer 90s tunnel wait.

Net: 2 devnets + 4 clients down to 1 devnet + 3 clients. DaemonClientIP is now validated in ClientSpec.Validate, the unownedIP comment says the true reason the address works, the doc examples name the right receiver, and the CHANGELOG bullet carries (#4243).

Verified: TestE2E_IPOwnershipProof_Enforced passes in CI (e2e shard 3, 142.52s, all four subtests) and locally (210s). TestE2E_IPOwnershipProof_ValidProof re-run locally as well, since DaemonClientIP touches shared client startup.

The PR description is updated to match — the old one described five tests and five devnets, and argued for the separate mismatch devnet.

[juan-malbeclabs]

Approving. I re-checked each thread against the amended commit rather than against the replies, and the code matches what they claim.

• One devnet. The mismatch test is now the fourth subtest on a third client of the shared devnet, and its comment says outright that the guard is client-side and flag-independent instead of implying enforcement coverage. That is the right trade: a client is one small container, a devnet is a ledger + manager + controller + cEOS.
• The duplicate is gone, and the one thing it carried that the sibling doesn't — "enforcement disturbs nothing downstream of the proof" — survives as the WaitForTunnelUp inside wildcard_pass_with_a_proof. Coverage went up while wall clock went down.
• unownedIP now names both properties that make the address work (publicly routable, and deliberately outside 9.128.0.0/9), so the next person doesn't move it into the container's own /24.
• DaemonClientIP validation landed in ClientSpec.Validate next to the CYOANetworkIPHostID range check, with the To4() half that catches an IPv6 literal — it fails at the same point every neighbouring address-shaped field does.

Two things I confirmed beyond reading the diff:

• The test runs, it doesn't merely compile: e2e (shard 3) reports --- PASS: TestE2E_IPOwnershipProof_Enforced (142.52s) with all four subtests passing.
• The strings the assertions hang on are the real ones — error.rs:222 for IpOwnershipProofRequired and connect.rs:418 for the address disagreement — so neither assertion can pass on an incidental failure.

The file header earning its length is worth calling out separately: stating why the reachable surface is two errors and only one of them testable here means the next person doesn't re-derive it, or worse, "fix" the gap by adding subtests that take a byte-identical program path.