e2e: cover RFC-27 proof enforcement with the feature flag set - #4243
Draft
elitegreg wants to merge 1 commit into
Draft
e2e: cover RFC-27 proof enforcement with the feature flag set#4243elitegreg wants to merge 1 commit into
elitegreg wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4205. Part of RFC-27 (
rfcs/rfc27-ip-verification.md, tracker #4194).Summary of Changes
require-ip-ownership-proofhad no e2e coverage at all. It was never set in any test, anddevnet.SetIPOwnershipProofFeatureFlaghad zero callers. Five tests now run with the flag set.access-pass setcall sites undere2e/name a--client-ip, so the case RFC-27 actually exists for was untested. A pass at the0.0.0.0PDA — the shape the shred-oracle issues — authorizes its payer for any routable address, which makes the proof the only thing bindingclient_ip. Covered both ways on one devnet: with a proof the user binds the observed address, without one the create is rejected.user createstill succeeds under enforcement while a client-paid one does not — the difference being the transaction payer, which is whatis_sentinelcompares.connectbinds its proof request to the address it provisions and refuses a proof for any other.ClientSpec.DaemonClientIPoverrides the daemon's-client-ip. Needed becausedoublezero connect --client-ipis 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_typeand 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 onlyIpOwnershipProofRequired(105) andIpProofEpochOutOfWindow(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.rshas 36 tests spanning every rejection condition, epoch window included, usingwarp_to_epoch— asolana-program-testfacility 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.SlotsPerEpochplumbed through the ledger entrypoint as--slots-per-epoch 32(silently ignored if the ledger volume already exists), plusIPVerifierSpecknobs forepoch_refresh_secs/max_epoch_age_secsso 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_UntrustedSignerin #4232, and it can only ever be a client-side assertion: the SDK'sverifycatches it before the transaction is built.Diff Breakdown
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 testse2e/docs/IP_VERIFIER_LOCAL_DEVNET.md— a "testing enforcement" section, including the two traps belowe2e/internal/devnet/client.go—ClientSpec.DaemonClientIPTesting 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, andWaitForTunnelUpreachesBGP 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:SentinelExemption— manager-sideuser createwith no proof succeeds under enforcement.ClientIpMismatchRefused— daemon set to an address the container does not own, so the verifier observes the real CYOA source:Also re-ran the three existing
TestE2E_IPOwnershipProoftests from #4232, sinceDaemonClientIPtouches shared client startup — all pass. (One run hit an unrelated controller container-start flake from leftover containers; clean aftermake e2e-test-cleanup.)Notes for the reviewer
doublezero connect --client-ipis deprecated and ignored, so the daemon flag is the only way to move the provisioned address.