Skip to content

sdk-go/stress: refuse CreateUser under RFC-27 enforcement and name the proof errors - #4252

Draft
elitegreg wants to merge 2 commits into
mainfrom
gm/sdk-go-ip-proof-gate
Draft

sdk-go/stress: refuse CreateUser under RFC-27 enforcement and name the proof errors#4252
elitegreg wants to merge 2 commits into
mainfrom
gm/sdk-go-ip-proof-gate

Conversation

@elitegreg

Copy link
Copy Markdown
Contributor

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

Stacked on #4243 — based on gm/e2e-ip-proof-enforcement. Must not merge before it. The diff shown here is against that branch.

Decision: option B, gate the tool

The issue offered two paths and recommended B. Two independent findings confirm it:

  • The addresses cannot be proven. The verifier signs only the address it observes a request originate from (crates/doublezero-ip-verifier/src/server.rs, resolve_client_ip). Executor.CreateUser's one production consumer is the device-stress orchestrator, whose addresses are ClientIPBase + idx — synthetic, and not addresses it originates traffic from. Binding an address the caller does not control is the exact thing RFC-27 exists to prevent, so porting the proof path would mean inventing a bypass for it.
  • The account layout has no room. genericInstruction.skipPermissionInject exists because the processor detects the optional tenant account by accounts.len(). Appending an Instructions sysvar account — which a proof requires — shifts that count and mis-classifies accounts.

Summary of Changes

  • CreateUser refuses when require-ip-ownership-proof is set, with a message naming the flag and what to do instead, rather than spending a transaction per user to collect IpOwnershipProofRequired (105). The flag is read from GlobalState once and cached behind a sync.Once, like the existing Permission PDA lookup. A failure to read global state warns and proceeds — the program is the authority on enforcement, and an unreadable account is not grounds for refusing to submit anything.
  • The ip_proof discriminant is emitted explicitly (12 → 13 bytes), so the Go payload equals the Rust user_create_args fixture byte for byte. TestBuildCreateUserInstruction drops its "minus the discriminant" exception. Relying on BorshDeserializeIncremental to default the field left the assertion unable to tell an intentionally absent proof from a truncated payload.
  • Named errors for the RFC-27 rejection classes. The custom-error map stopped at 90, so every RFC-27 rejection rendered as unknown error code 105. Codes 91–118 gained names, and ClassifyProgramError annotates a transaction error with a comparable ProgramError so callers can match it with errors.Is; Unwrap() []error keeps the original jsonrpc.RPCError reachable.
  • CLAUDE.md no longer describes the Go SDK as read-only.

A pre-existing bug found on the way, deliberately not fixed here

ByteReader.ReadU128 assigns Uint128.High from the first eight encoded bytes, but Borsh writes a u128 little-endian — those are the low half. Confirmed against the fixture: global_state.json has AccountIndex: 42 and global_state.bin encodes it as 2a 00 00 … in the first eight bytes, which land in .High. The two fields are named the wrong way round.

No production Go reads .High/.Low — only the deserializer writes them — but client_test.go bakes in the swapped convention (Uint128{High: 12, Low: 0}), so correcting it touches those tests and belongs in its own change. This PR isolates the wart behind a documented Uint128.Lo64() accessor and does not rename anything. Worth its own issue.

Testing Verification

  • TestBuildCreateUserInstruction now asserts the full Rust fixture with no trailing-byte exception.
  • TestCreateUserRefusesWhenProofRequired: with the flag set, nothing is submitted (sentTransactions empty), the derived user PDA is still returned so a caller can correlate the refusal, and a second attempt re-uses the cached flags rather than re-reading global state.
  • TestCreateUserProceedsWhenProofNotRequired: with the flag clear, the create is submitted — so the guard cannot pass by refusing everything.
  • TestGlobalStateIsFeatureEnabled round-trips through the real deserializer rather than constructing a Uint128 by hand, and checks that bit 1 is not mistaken for bit 2.
  • TestClassifyProgramErrorMapsIPOwnershipProofRequired asserts the named match, the absence of a match against a sibling class, and that the underlying jsonrpc.RPCError survives.
  • Whole-repo go build ./... and the full smartcontract/sdk/go/... plus tools/stress/... suites pass; golangci-lint reports 0 issues on the changed package.

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.

Two 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: 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.

Everything that can share a devnet does, because a devnet is the expensive part
of an e2e test and an extra client is one small container. The first four
outcomes are subtests over one devnet with three clients; only the mismatch case
needs a second, since DaemonClientIP is fixed when the container starts.
…e proof errors

Executor.CreateUser hand-packs a 12-byte payload with no ip_proof discriminant,
no Instructions sysvar account and no Ed25519 instruction. It works today only
because UserCreateArgs is BorshDeserializeIncremental and a None proof is
accepted while require-ip-ownership-proof is clear. The moment the flag is set
for an environment every creation this path submits fails with
IpOwnershipProofRequired (105); the payer is not the sentinel authority, so the
exemption does not apply.

Teaching the builder to carry a proof would not help. The verifier signs only
the address it observes a request originate from, and the one production
consumer is the device-stress orchestrator, whose ClientIPBase + idx addresses
are synthetic. Binding an address the caller does not control is the exact thing
RFC-27 exists to prevent, so the tool is gated instead: CreateUser reads the
feature flag once, caches it, and refuses with a message naming the flag and
what to do about it, rather than spending a transaction per user to collect the
same rejection. A failure to read global state warns and proceeds -- the program
is the authority on enforcement, and an unreadable account is not grounds for
refusing to submit anything.

The ip_proof discriminant is now emitted explicitly, so the Go payload equals
the Rust fixture byte for byte and the test drops its "minus the discriminant"
exception. Relying on incremental defaulting left the assertion unable to tell
an intentionally absent proof from a truncated payload.

The custom-error map stopped at 90, so every RFC-27 rejection rendered as
"unknown error code 105". Codes 91-118 gained names, and ClassifyProgramError
annotates a transaction error with a named ProgramError so callers can match it
with errors.Is while the original RPC error stays reachable.

Uint128.Lo64 isolates a pre-existing wart: ByteReader.ReadU128 fills .High from
the first eight encoded bytes, which Borsh little-endian makes the low half, so
the fields are named the wrong way round. Fixtures and existing tests bake in
that convention, so the accessor documents it in one place rather than renaming
fields here.

Also corrects CLAUDE.md, which still described the Go SDK as read-only.
@elitegreg
elitegreg force-pushed the gm/e2e-ip-proof-enforcement branch from 632189a to abc59f7 Compare September 1, 2026 19:58
Base automatically changed from gm/e2e-ip-proof-enforcement to main September 1, 2026 22:44
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