diff --git a/CHANGELOG.md b/CHANGELOG.md index e6befaf218..25caaa4435 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. - CI - `.cursor/BUGBOT.md` and `.github/copilot-instructions.md` now tell Bugbot and Copilot to read the nearest sibling, flag a path that skips a zero or a duplicate, and assert a specific error and the exact log line at the expected index. Onchain checks apply only when the repository has onchain code. The eight path-scoped files under `.github/instructions/` are removed so Copilot reads only the repo-wide file. (#4247) - E2E/QA + - New e2e coverage for RFC-27 proof enforcement with `require-ip-ownership-proof` set: the working path still reaches BGP, a client with no verifier to reach is rejected with `IpOwnershipProofRequired`, a wildcard (`0.0.0.0`) access pass binds `client_ip` only when a proof is attached, the sentinel authority stays exempt so the oracle path keeps working, and `connect` refuses a proof whose address disagrees with the one it provisions. - Remove `TestQA_MulticastSettlement`. It funded a seat through `doublezero-solana shreds pay`, which is going away. The agent seat-pay RPC now returns Unimplemented if something still calls it. Unused settlement helpers go with the test. (#4248) ## [v0.38.0](https://github.com/malbeclabs/doublezero/compare/client/v0.37.0...client/v0.38.0) - 2026-08-28 diff --git a/e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md b/e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md index 16d5544fce..2550cbe752 100644 --- a/e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md +++ b/e2e/docs/IP_VERIFIER_LOCAL_DEVNET.md @@ -70,6 +70,30 @@ attaches a real proof. Two knobs cover the cases that need something else: `e2e/ip_ownership_proof_test.go` uses all three paths. +### Testing enforcement + +`devnet.SetIPOwnershipProofFeatureFlag(ctx, true)` sets `require-ip-ownership-proof`, which changes +exactly one thing in the program: whether a *missing* proof is an error. A supplied proof is +validated in full either way, so most proof failures are testable with the flag clear. + +Two things to know before writing a "flag on rejects everything" test: + +- **The manager is the sentinel authority** in a local devnet (`smartcontract_init.go` runs + `authority set --sentinel-authority me`), and the sentinel may create a user without a proof. So + `doublezero user create` from the manager still succeeds under enforcement — the rejection only + shows up on a create paid for by someone else, which is what `doublezero connect` on a client + does. `is_sentinel` compares the transaction payer. +- **Most bad-proof cases never reach the chain.** The Rust SDK pre-flights version, payer, + `client_ip`, `user_type` and the signature before building the transaction, and `connect` refuses + an address disagreement before that. Of the program's proof errors only + `IpOwnershipProofRequired` (105) and `IpProofEpochOutOfWindow` (110) are reachable end to end, + and 110 needs a ledger epoch the devnet never advances past 0. The rest have program-level + coverage in `user_ip_proof_test.rs`. + +`e2e/ip_ownership_proof_enforcement_test.go` covers the flag-set cases, including a wildcard access +pass — a pass created with no `--client-ip`, landing at the `0.0.0.0` PDA — which is the case +RFC-27 exists for. + ## Poking at it ```bash diff --git a/e2e/internal/devnet/client.go b/e2e/internal/devnet/client.go index 1e2448fdba..5eb63d00da 100644 --- a/e2e/internal/devnet/client.go +++ b/e2e/internal/devnet/client.go @@ -65,6 +65,12 @@ type ClientSpec struct { // RFC-27 proof even in a devnet running a verifier. NoIPVerifier bool + // DaemonClientIP overrides the address the daemon provisions, which is otherwise this client's + // CYOA address. Set it to an address the container does not own and the verifier observes the + // real CYOA source while `connect` binds the override, which is the disagreement `connect` + // refuses to provision through. + DaemonClientIP string + // EnableQAAgent starts the QA agent inside the client container for local QA testing. EnableQAAgent bool // QAAgentPort is the port the QA agent listens on inside the container (default: 7009). @@ -223,7 +229,11 @@ func (c *Client) Start(ctx context.Context) error { if c.Spec.LatencyProbeTunnelEndpoints { extraArgs = append(extraArgs, "-latency-probe-tunnel-endpoints") } - extraArgs = append(extraArgs, "-client-ip", clientCYOAIP) + daemonClientIP := clientCYOAIP + if c.Spec.DaemonClientIP != "" { + daemonClientIP = c.Spec.DaemonClientIP + } + extraArgs = append(extraArgs, "-client-ip", daemonClientIP) // Determine QA agent port if enabled. qaAgentPort := c.Spec.QAAgentPort diff --git a/e2e/ip_ownership_proof_enforcement_test.go b/e2e/ip_ownership_proof_enforcement_test.go new file mode 100644 index 0000000000..9a9a20d415 --- /dev/null +++ b/e2e/ip_ownership_proof_enforcement_test.go @@ -0,0 +1,245 @@ +//go:build e2e + +package e2e_test + +import ( + "testing" + "time" + + "github.com/malbeclabs/doublezero/e2e/internal/devnet" + "github.com/stretchr/testify/require" +) + +// RFC-27 enforcement: the `require-ip-ownership-proof` feature flag set. +// +// The sibling file covers the flag-clear outcomes, where a missing proof is tolerated. Here the +// flag is on, which changes exactly one thing in the program: whether a *missing* proof is an +// error (`ip_proof.rs`, the `None` arm). A supplied proof is validated in full either way. +// +// What is worth testing at this level is narrow, and deliberately so. The serviceability program's +// own `user_ip_proof_test.rs` already covers every rejection condition against a program-test +// runtime, and the Rust SDK pre-flights version, payer, client_ip, user_type and the signature +// before it builds a transaction — so those rejections can never reach the chain through the real +// CLI. Of the program's proof errors only `IpOwnershipProofRequired` (105) and +// `IpProofEpochOutOfWindow` (110) are reachable end to end, and 110 needs a ledger epoch that a +// devnet never advances past 0. So 105 is the one onchain rejection these tests can assert, and +// the rest of the value here is in the integration: a real verifier, a real ledger, a real tunnel. + +// The working path still works with enforcement on: proof obtained, attached, accepted, tunnel up. +func TestE2E_IPOwnershipProof_EnforcedWithValidProof(t *testing.T) { + t.Parallel() + + dn, _, client, log := setupIPProofDevnet(t, devnet.IPVerifierSpec{}, devnet.ClientSpec{ + CYOANetworkIPHostID: 100, + }) + + log.Info("==> Enabling require-ip-ownership-proof") + require.NoError(t, dn.SetIPOwnershipProofFeatureFlag(t.Context(), true)) + + out := connectIBRLForProofTest(t, log, dn, client) + + require.Contains(t, out, "IP ownership verified for "+client.CYOANetworkIP) + require.Contains(t, out, "✅ User Provisioned") + + // The whole point of asserting BGP rather than stopping at account creation: enforcement must + // not disturb anything downstream of the proof. + require.NoError(t, client.WaitForTunnelUp(t.Context(), 90*time.Second), + "a user created under enforcement must still reach BGP") +} + +// The enforcement moment: with the flag on, a client that cannot reach a verifier is refused. +// +// This is the one rejection in this file that is genuinely the program's. The CLI attaches no +// proof, so nothing is caught client-side, and `create_user` fails with +// `DoubleZeroError::IpOwnershipProofRequired` — custom program error 105 (0x69). +func TestE2E_IPOwnershipProof_EnforcedRejectsMissingProof(t *testing.T) { + t.Parallel() + + dn, _, client, log := setupIPProofDevnet(t, devnet.IPVerifierSpec{}, devnet.ClientSpec{ + CYOANetworkIPHostID: 100, + NoIPVerifier: true, + }) + + log.Info("==> Enabling require-ip-ownership-proof") + require.NoError(t, dn.SetIPOwnershipProofFeatureFlag(t.Context(), true)) + + setAccessPass(t, dn, client) + + log.Info("==> Connecting with no verifier configured") + out, err := client.Exec(t.Context(), []string{"bash", "-c", "doublezero connect ibrl 2>&1"}) + output := string(out) + log.Info("==> Connect output", "output", output) + + require.Error(t, err, "a create with no proof must be refused while the flag is set") + // Assert the specific refusal, not merely that connect failed: a test that passes because + // connect broke for an unrelated reason would be worse than no test at all. + require.Contains(t, output, "An IP ownership proof is required to create a user", + "the refusal must be the program's IpOwnershipProofRequired, not an incidental failure") + require.NotContains(t, output, "✅ User Provisioned") + + requireNoUserForIP(t, dn, client.CYOANetworkIP) +} + +// The case RFC-27 exists for. +// +// A wildcard access pass — stored at the 0.0.0.0 PDA, which is the shape the shred-oracle issues — +// authorizes its payer for *any* routable address. Without a proof the program would let that payer +// squat the User PDA of an address they do not control and point device tunnel provisioning at a +// third party. For a specific-IP pass the issuing authority already chose the IP, so the proof is +// redundant there; here it is the only thing binding client_ip. +// +// Two clients on one devnet, because whether a client has a verifier is fixed at container start. +func TestE2E_IPOwnershipProof_WildcardAccessPass(t *testing.T) { + t.Parallel() + + dn, device, verified, log := setupIPProofDevnet(t, devnet.IPVerifierSpec{}, devnet.ClientSpec{ + CYOANetworkIPHostID: 100, + }) + + log.Info("==> Enabling require-ip-ownership-proof") + require.NoError(t, dn.SetIPOwnershipProofFeatureFlag(t.Context(), true)) + + // A second client with no verifier to reach, to show the same wildcard pass is refused without + // a proof. + unverified, err := dn.AddClient(t.Context(), devnet.ClientSpec{ + CYOANetworkIPHostID: 101, + NoIPVerifier: true, + }) + require.NoError(t, err) + require.NoError(t, unverified.WaitForLatencyResults(t.Context(), device.ID, 75*time.Second)) + log.Info("--> Second client added", "clientIP", unverified.CYOANetworkIP, "pubkey", unverified.Pubkey) + + // No --client-ip: the pass lands at the UNSPECIFIED (0.0.0.0) PDA and admits any address. + for _, c := range []*devnet.Client{verified, unverified} { + _, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", + "doublezero access-pass set --accesspass-type prepaid --epochs max --user-payer " + c.Pubkey}) + require.NoError(t, err) + } + + if !t.Run("with_a_proof", func(t *testing.T) { + log.Info("==> Connecting the verified client on a wildcard pass") + out, err := verified.Exec(t.Context(), []string{"bash", "-c", "doublezero connect ibrl 2>&1"}) + output := string(out) + log.Info("==> Connect output", "output", output) + require.NoError(t, err, "connect failed: %s", output) + + require.Contains(t, output, "IP ownership verified for "+verified.CYOANetworkIP) + require.Contains(t, output, "✅ User Provisioned") + + // The pass named no address, so the proof is what bound this one. + users, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", "doublezero user list"}) + require.NoError(t, err) + require.Contains(t, string(users), verified.CYOANetworkIP, + "the user must be bound to the address the verifier observed") + }) { + t.Fail() + } + + if !t.Run("without_a_proof", func(t *testing.T) { + log.Info("==> Connecting the unverified client on a wildcard pass") + out, err := unverified.Exec(t.Context(), []string{"bash", "-c", "doublezero connect ibrl 2>&1"}) + output := string(out) + log.Info("==> Connect output", "output", output) + + require.Error(t, err, "a wildcard pass must not admit an unproven address under enforcement") + require.Contains(t, output, "An IP ownership proof is required to create a user") + requireNoUserForIP(t, dn, unverified.CYOANetworkIP) + }) { + t.Fail() + } +} + +// The sentinel exemption, which is what keeps enforcement from breaking the shred-oracle. +// +// The oracle provisions multicast publishers owned by validators, for addresses the verification +// service never sees a request from, so there is no proof it could obtain. The program waives the +// *requirement* for a creation paid for by `globalstate.sentinel_authority_pk`. +// +// In this devnet the manager is that authority (`smartcontract_init.go` runs +// `authority set --sentinel-authority me`), and `doublezero user create` never attaches a proof at +// all, so a manager-side create is the exemption in action. The contrast with +// TestE2E_IPOwnershipProof_EnforcedRejectsMissingProof is the transaction payer, which is what +// `is_sentinel` compares. +func TestE2E_IPOwnershipProof_SentinelExemption(t *testing.T) { + t.Parallel() + + dn, device, _, log := setupIPProofDevnet(t, devnet.IPVerifierSpec{}, devnet.ClientSpec{ + CYOANetworkIPHostID: 100, + }) + + log.Info("==> Enabling require-ip-ownership-proof") + require.NoError(t, dn.SetIPOwnershipProofFeatureFlag(t.Context(), true)) + + // An address the manager owns a pass for. It has no container behind it; this test is about + // whether the creation is admitted, not about tunnels. + const sentinelUserIP = "9.0.0.9" + + _, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", + "doublezero access-pass set --accesspass-type prepaid --epochs max --client-ip " + + sentinelUserIP + " --user-payer me"}) + require.NoError(t, err) + + log.Info("==> Creating a user as the sentinel authority, with no proof") + out, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", + "doublezero user create --device " + device.Spec.Code + " --client-ip " + sentinelUserIP + " 2>&1"}) + log.Info("==> User create output", "output", string(out)) + require.NoError(t, err, "the sentinel authority must be exempt from the proof requirement: %s", string(out)) + + users, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", "doublezero user list"}) + require.NoError(t, err) + require.Contains(t, string(users), sentinelUserIP) +} + +// A proof for an address other than the one being provisioned is refused. +// +// The daemon is told to provision an address this container does not own, so the CLI cannot bind +// its proof request to it (`probe_source_binding`) and the request leaves from the real CYOA +// address. The verifier signs what it observes, and `connect` refuses rather than binding an +// address it has no proof for. +// +// This asserts the client-side guard on purpose. The equivalent onchain error +// (`IpProofClientIpMismatch`, 108) is unreachable through the SDK, which pre-flights the same +// comparison before building a transaction; the program-level case is covered by +// `test_proof_for_a_different_client_ip_is_rejected`. +func TestE2E_IPOwnershipProof_ClientIpMismatchRefused(t *testing.T) { + t.Parallel() + + // Routable, in the CYOA range, and not an address this container holds. + const unownedIP = "9.0.0.7" + + dn, _, client, log := setupIPProofDevnet(t, devnet.IPVerifierSpec{}, devnet.ClientSpec{ + CYOANetworkIPHostID: 100, + DaemonClientIP: unownedIP, + }) + + log.Info("==> Enabling require-ip-ownership-proof") + require.NoError(t, dn.SetIPOwnershipProofFeatureFlag(t.Context(), true)) + + // The pass has to cover the address the daemon reports, or connect stops on the pass instead. + _, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", + "doublezero access-pass set --accesspass-type prepaid --epochs max --client-ip " + + unownedIP + " --user-payer " + client.Pubkey}) + require.NoError(t, err) + + log.Info("==> Connecting with a daemon client IP this host does not own", + "provisioning", unownedIP, "observable", client.CYOANetworkIP) + out, err := client.Exec(t.Context(), []string{"bash", "-c", "doublezero connect ibrl 2>&1"}) + output := string(out) + log.Info("==> Connect output", "output", output) + + require.Error(t, err, "connect must refuse a proof for an address other than the one it binds") + require.Contains(t, output, "The verification service observed this host at", + "the refusal must be the address disagreement, not an incidental failure") + require.NotContains(t, output, "✅ User Provisioned") + + requireNoUserForIP(t, dn, unownedIP) +} + +// requireNoUserForIP asserts a rejected creation left nothing onchain. +func requireNoUserForIP(t *testing.T, dn *devnet.Devnet, clientIP string) { + t.Helper() + users, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", "doublezero user list"}) + require.NoError(t, err) + require.NotContains(t, string(users), clientIP, + "no user may exist for a client whose creation was rejected") +}