geolocation: Go SDK — GeoProbe types and client methods

[ben-dz]

RFC Reference: rfc16-geolocation-verification.md (as updated by PR #2950)
Parent Issue: #2943
Depends on: #2952 (Geolocation Program — GeoProbe half)
Split from: #2953

Goal: Create the GeoProbe portion of the Go SDK for the Geolocation program — types, deserialization, PDA derivation, instruction builders, and client methods.

Scope

• New Go package smartcontract/sdk/go/geolocation/
• Go structs matching Rust serialization layout:
  • GeolocationProgramConfig
  • GeoProbe
• PDA derivation functions:
  • DeriveProgramConfigPDA()
  • DeriveGeoProbePDA() using seeds ["doublezero", "probe", code]
• Instruction builder functions for GeoProbe instructions:
  • InitProgramConfig
  • UpdateProgramConfig
  • CreateGeoProbe
  • UpdateGeoProbe
  • DeleteGeoProbe
  • AddParentDevice
  • RemoveParentDevice
• Client methods:
  • GetProgramConfig(), GetGeoProbes(), GetGeoProbeByCode()
  • Transaction builders for all GeoProbe instructions
• Deserialization for GeolocationProgramConfig and GeoProbe account types
• Unit tests for PDA derivation (must match Rust), serialization round-trip

Key files to create

• smartcontract/sdk/go/geolocation/client.go
• smartcontract/sdk/go/geolocation/constants.go
• smartcontract/sdk/go/geolocation/state.go (ProgramConfig + GeoProbe structs)
• smartcontract/sdk/go/geolocation/pda.go
• smartcontract/sdk/go/geolocation/deserialize.go
• smartcontract/sdk/go/geolocation/executor.go
• smartcontract/sdk/go/geolocation/rpc.go
• GeoProbe instruction builder files
• smartcontract/sdk/go/geolocation/*_test.go

[ben-dz]

Architecture review findings:

• Critical: 0
• High: 2
• Medium: 3
• Low: 3

Architecture Review

Critical

None

High

• [Code Duplication] executor.go is a verbatim copy of telemetry's executor.go: The geolocation executor.go (185 lines) is character-for-character identical to smartcontract/sdk/go/telemetry/executor.go (the transaction-execution portion, lines 1-185), differing only in the package declaration. The same duplication pattern also exists for rpc.go (the RPCClient interface). With three packages now carrying independent copies (telemetry, geolocation, and the serviceability executor having a third divergent version), this creates a maintenance hazard where a bug fix or improvement to one copy (e.g., context-cancellation handling, retry logic) will not propagate to the others.

  • Location: smartcontract/sdk/go/geolocation/executor.go:1-185 vs smartcontract/sdk/go/telemetry/executor.go:1-185
  • Recommendation: This matches the existing pattern (telemetry did it first), so it is acceptable for this PR. However, consider extracting the shared executor and RPCClient interface into a common internal package (e.g., smartcontract/sdk/go/internal/solexec) as a follow-up. File a tracking issue so this tech debt does not grow further.

• [Resilience] waitForSignatureVisible does not respect context cancellation: The polling loop in waitForSignatureVisible uses time.Sleep(250ms) which is not interruptible by context cancellation. If the caller's context is cancelled (e.g., shutdown signal), the function will continue sleeping until the timeout expires. The waitForTransactionFinalized function correctly uses select with ctx.Done(), so this is an inconsistency. Note: this is inherited from the telemetry executor and is not a new bug introduced by this PR, but it is worth flagging since this is new code being added.

  • Location: smartcontract/sdk/go/geolocation/executor.go:131-145
  • Recommendation: Replace time.Sleep(250 * time.Millisecond) with a select on ctx.Done() and time.After(250ms), consistent with how waitForTransactionFinalized works. Consider fixing it in both telemetry and geolocation simultaneously.

Medium

• [API Design] NewExecutor is exported but returns unexported type *executor: The function NewExecutor is exported (capital N) but returns *executor (lowercase). In Go, this is a known antipattern -- callers outside the package can call it but cannot declare a variable of the returned type, store it in a struct, or pass it to functions expecting *executor. Currently NewExecutor is only called internally from client.go within the same package, so the export is unnecessary.

  • Location: smartcontract/sdk/go/geolocation/executor.go:38
  • Recommendation: Either make the constructor unexported (newExecutor) since it is only used internally, or export the struct type as well. The telemetry SDK has this same issue, but since this is new code, it is a good time to clean it up. The ExecutorOption type and WithWaitForVisibleTimeout are also exported but have no external consumers.

• [API Design] RPCClient interface includes unused SendTransaction method: The RPCClient interface declares SendTransaction (without opts), but no code in the geolocation package calls it -- only SendTransactionWithOpts is used (in executor.go:106). This forces mock implementations to satisfy an extra method they never need.

  • Location: smartcontract/sdk/go/geolocation/rpc.go:12
  • Recommendation: Remove SendTransaction from the interface. It is also unused in the telemetry RPCClient, but the geolocation SDK is new code and has the chance to get it right. The mock in main_test.go already works around this by embedding the interface to get nil-panic implementations of unused methods.

• [Testability] No executor tests for the geolocation package: The telemetry SDK has comprehensive executor tests (executor_test.go with 11 test functions). The geolocation package has zero executor-level tests. Since the executor is a verbatim copy, the telemetry tests do provide indirect confidence, but if the two diverge in the future, the geolocation package would have no safety net.

  • Location: smartcontract/sdk/go/geolocation/ (missing executor_test.go)
  • Recommendation: Either (a) extract the shared executor into a common package with its own tests (preferred), or (b) add executor tests to the geolocation package.

Low

• [Error Handling] Validation errors use fmt.Errorf instead of sentinel errors: All Validate() methods in instruction configs return ad-hoc error strings. This means callers cannot programmatically check for specific validation failures using errors.Is. This is fine for now since validation errors are typically programmer mistakes caught during development.

  • Location: smartcontract/sdk/go/geolocation/init_program_config.go:16-23 (and all other instruction config files)
  • Recommendation: No action needed for this PR.

• [API Design] Serialize methods accept io.Writer while Deserialize accepts []byte: The serialization/deserialization API is asymmetric. This is consistent with the telemetry SDK's approach and works fine in practice.

  • Location: smartcontract/sdk/go/geolocation/state.go:27,47
  • Recommendation: No action needed. Matches the established pattern.

• [Documentation] Inconsistent comment style on WithGeolocationProgramID: The comment says "Configure the geolocation program ID." while Go convention starts with the function name. This matches existing comments in the file.

  • Location: smartcontract/sdk/go/client.go:101
  • Recommendation: No action needed for this PR.

Summary

The geolocation Go SDK is well-structured and follows the established patterns of the telemetry SDK closely. The API design, error handling, separation of concerns, and test coverage for the domain-specific code (state serialization, PDA derivation, instruction building, client methods) are all solid. The primary concern is the verbatim duplication of the ~185-line executor and the RPCClient interface across packages, which creates future maintenance risk. The waitForSignatureVisible function's lack of context cancellation support is inherited from telemetry but worth fixing in this new code. The backward compatibility of the client.go change is clean -- the Geolocation field is additive and opt-in via WithGeolocationProgramID.

[ben-dz]

Security review findings:

• Critical: 0
• High: 0
• Medium: 3
• Low: 4

Security Review

Critical

None

High

None

Medium

• [Deserialization - Missing Account Owner Verification]: When fetching accounts via GetProgramConfig, GetGeoProbeByCode, and GetGeoProbes, the SDK verifies the account type discriminator (first byte) but does not verify that the fetched account is actually owned by the expected geolocation program. A malicious or misconfigured RPC endpoint, or an account at a PDA that happens to exist under a different program, could return data with a matching discriminator byte that gets deserialized as a valid geolocation account. While the PDA derivation uses the program ID as a seed (making collisions extremely unlikely in practice), the defense-in-depth pattern of checking account.Value.Owner == programID is missing.

  • Location: smartcontract/sdk/go/geolocation/client.go:46-68 (GetProgramConfig), lines 71-93 (GetGeoProbeByCode), lines 96-123 (GetGeoProbes)
  • Recommendation: After fetching the account, verify account.Value.Owner == c.executor.programID before deserializing the data. Note: this is a pre-existing pattern across the other SDK packages in this repo (serviceability, telemetry) which also lack this check, so it is consistent with the codebase. However, it remains a defense-in-depth gap.

• [Deserialization - Potential OOM via Crafted Length Prefix]: The GeoProbe.Deserialize method uses gagliardetto/binary BorshDecoder to decode the Code (string) and ParentDevices (slice) fields. Borsh strings and slices are length-prefixed with a 4-byte LE uint32. A crafted binary payload could specify a length prefix of up to ~4GB for Code or ParentDevices, potentially causing the decoder to attempt a large memory allocation before bounds checking occurs. The post-decode checks (len(g.Code) > MaxCodeLength and len(g.ParentDevices) > MaxParentDevices) only trigger after the allocation has already happened. In practice, Solana account data is limited to 10MB, and the RPC response size caps this further, but if the deserialization functions are used on untrusted input outside the RPC context, this could be exploited.

  • Location: smartcontract/sdk/go/geolocation/state.go:119-161 (GeoProbe.Deserialize)
  • Recommendation: Consider validating the remaining buffer length against the declared length prefix before decoding, or set an explicit maximum on the decoder. The post-decode checks for MaxCodeLength and MaxParentDevices are good but occur after allocation. In the context of Solana account data (max ~10MB), this is acceptable risk but worth documenting.

• [Transaction Safety - Delete Instruction Payer Not Writable]: In BuildDeleteGeoProbeInstruction, the Payer account is marked as IsWritable: false. When deleting a Solana account, the runtime typically refunds the account's rent-exempt lamports to a designated recipient. If the Payer is intended to receive the rent refund (which is the common pattern), it must be writable. Without seeing the corresponding Rust program, this may or may not be a bug depending on the program's implementation, but it deviates from the pattern seen in other instructions where the payer paying rent (init, create) is correctly marked writable.

  • Location: smartcontract/sdk/go/geolocation/delete_geo_probe.go:44
  • Recommendation: Verify against the onchain program's account constraints. If the payer receives the rent refund, change to IsWritable: true. If another account receives the refund, ensure that account is included and marked writable.

Low

• [Context Cancellation - waitForSignatureVisible Ignores Context]: The waitForSignatureVisible function uses a time.Sleep(250ms) in a busy loop without checking ctx.Done(). If the caller cancels the context, the function will not respond until the next RPC call propagates the context error. This means the function can block for up to 250ms after context cancellation. This is a pre-existing pattern (identical code exists in the telemetry executor).

  • Location: smartcontract/sdk/go/geolocation/executor.go:131-145
  • Recommendation: Replace time.Sleep(250 * time.Millisecond) with a select on ctx.Done() and time.After(250ms).

• [Secrets Exposure - Signer Getter Returns Pointer to Private Key]: The Client.Signer() method returns a *solana.PrivateKey pointer, giving callers direct access to the private key material. While this follows the same pattern as the telemetry SDK and is necessary for the SDK's functionality, it increases the surface area through which the key could be inadvertently logged, serialized, or passed to untrusted code. No actual logging of the key was observed.

  • Location: smartcontract/sdk/go/geolocation/client.go:38-43
  • Recommendation: No immediate action required, as this matches existing patterns.

• [Input Validation - MustPublicKeyFromBase58 Panics on Invalid Input]: In client.go, the WithGeolocationProgramID option function uses solana.MustPublicKeyFromBase58(programID) which will panic if provided an invalid base58 string. This is consistent with the existing codebase pattern and the "Must" prefix convention in Go.

  • Location: smartcontract/sdk/go/client.go:102-106
  • Recommendation: No action needed.

• [Deserialization - No Trailing Bytes Validation]: The Deserialize methods on GeolocationProgramConfig and GeoProbe do not verify that all input bytes were consumed. Extra trailing bytes are silently ignored, as demonstrated by the test TestSDK_Geolocation_DeserializeGeoProbe_ExtraTrailingBytes. This is an intentionally tested forward-compatibility strategy.

  • Location: smartcontract/sdk/go/geolocation/state.go:47-65 (ProgramConfig), lines 119-161 (GeoProbe)
  • Recommendation: Consider logging a warning when trailing bytes remain after deserialization, or add a version check. Current behavior is reasonable for forward compatibility.

Summary

The geolocation Go SDK is well-structured with solid input validation on instruction builders, proper discriminator checking on deserialization, and correct signer verification before transaction execution. The code closely follows established patterns from the telemetry and serviceability SDKs. The most notable findings are the missing account owner verification during deserialization (a defense-in-depth gap shared with other SDKs in the repo), the potential for the delete instruction's payer writable flag to be incorrect (requires verification against the onchain program), and a minor OOM vector in deserialization from crafted length prefixes that is mitigated by Solana's account size limits in practice. No critical or high-severity issues were identified.

[ben-dz]

Architecture review findings:

• Critical: 0
• High: 2
• Medium: 3
• Low: 4

Architecture Review

Critical

None

High

• [Code Duplication] Executor is a near-exact copy of the telemetry executor: The geolocation executor.go (184 lines) is a character-for-character duplicate of telemetry/executor.go (184 lines). This includes the full ExecuteTransactions flow, waitForSignatureVisible, waitForTransactionFinalized, the ErrNoPrivateKey/ErrNoProgramID sentinel errors, and the ExecutorOption/ExecuteTransactionOptions types. This is a maintenance hazard -- any bug fix or improvement must be applied to both files independently.

  • Location: smartcontract/sdk/go/geolocation/executor.go:1-185 vs smartcontract/sdk/go/telemetry/executor.go:1-185
  • Recommendation: Extract a shared executor package (e.g., smartcontract/sdk/go/internal/executor) that both telemetry and geolocation import. If a shared package is undesirable for dependency reasons, at minimum document that these two files must be kept in sync.

• [RPCClient Interface] SendTransaction method is declared but never used: The RPCClient interface in rpc.go includes SendTransaction, but the executor exclusively uses SendTransactionWithOpts. This mirrors the same dead method in telemetry's RPCClient interface.

  • Location: smartcontract/sdk/go/geolocation/rpc.go:12
  • Recommendation: Remove SendTransaction from the geolocation RPCClient interface.

Medium

• [Context Handling] waitForSignatureVisible does not respect context cancellation: The polling loop uses time.Sleep(250ms) without checking ctx.Done(). Inherited from the telemetry executor.

  • Location: smartcontract/sdk/go/geolocation/executor.go:131-145
  • Recommendation: Replace time.Sleep with a select on ctx.Done() and time.After.

• [Backward Compatibility] Geolocation client is nil unless program ID is explicitly provided: Consistent with RevenueDistribution but unlike serviceability/telemetry.

  • Location: smartcontract/sdk/go/client.go:66-68
  • Recommendation: Acceptable for now; add default program ID when deployed to testnet.

• [Error Handling] Validation errors use fmt.Errorf instead of sentinel errors.

  • Location: All Validate() methods
  • Recommendation: Define sentinel validation errors for programmatic matching.

Low

• [Naming] NewExecutor exports a function returning unexported type (consistent with telemetry).
• [Naming] Inconsistent parameter naming in RPCClient interface.
• [Consistency] DeleteGeoProbe takes ProbePK directly vs code in other operations.
• [Documentation] Instruction builders lack doc comments.

Summary

The geolocation SDK is well-structured, closely follows the established telemetry SDK patterns, and provides good test coverage for read paths, serialization, and instruction building. The two primary concerns are (1) the wholesale duplication of the executor and (2) the lack of tests for the write/transaction execution path. The remaining findings are mostly about polish.

[ben-dz]

Security review findings:

• Critical: 0
• High: 0
• Medium: 2
• Low: 4

Security Review

Critical

None

High

None

Medium

• [Denial of Service - Unbounded polling loop]: The waitForTransactionFinalized function in the executor contains an infinite for {} loop that only terminates when the transaction reaches ConfirmationStatusFinalized or the context is cancelled. If the caller passes a context without a deadline/timeout, this loop will poll indefinitely.

  • Location: smartcontract/sdk/go/geolocation/executor.go:150-171
  • Recommendation: Add an explicit maximum timeout independent of the caller's context. Note: This is a pre-existing pattern inherited from the telemetry SDK executor, so both should be fixed together.

• [Denial of Service - Borsh deserialization of ParentDevices slice]: During GeoProbe.Deserialize, the Borsh decoder reads a 4-byte length prefix for the ParentDevices slice and allocates accordingly before the MaxParentDevices bounds check on line 148. A crafted input with a large length prefix could cause a massive allocation before the bounds check fires.

  • Location: smartcontract/sdk/go/geolocation/state.go:145-149
  • Recommendation: Validate the length prefix from the raw bytes before calling dec.Decode, or use custom deserialization that reads the count first and checks it before allocating. Same concern for Code string at line 139-143.

Low

• [Information Exposure - Signer accessor returns private key reference]: The Client.Signer() method returns a pointer to the solana.PrivateKey. Follows telemetry SDK pattern.

  • Location: smartcontract/sdk/go/geolocation/client.go:38-43
  • Recommendation: Consider whether the accessor is strictly necessary in the public API.

• [Missing validation - No account owner check on deserialized accounts]: The code validates the account type discriminator but does not verify that the returned account is owned by the geolocation program.

  • Location: smartcontract/sdk/go/geolocation/client.go:46-68 and client.go:71-93
  • Recommendation: Check account.Value.Owner equals expected programID before deserializing.

• [Deserialization leniency - Trailing bytes accepted silently]: The Borsh deserialization does not verify that all bytes have been consumed. Intentional for forward compatibility.

  • Location: smartcontract/sdk/go/geolocation/state.go:47-65 and state.go:119-161
  • Recommendation: Document this as intentional or log a warning.

• [Inconsistent preflight skip]: Some methods pass SkipPreflight: true while others do not. The inconsistency may not be deliberate.

  • Location: smartcontract/sdk/go/geolocation/client.go:135-136, client.go:171-172, client.go:224-225
  • Recommendation: Standardize or document the preflight behavior.

Summary

The geolocation Go SDK is well-structured and follows established patterns from the existing telemetry SDK. The most significant concern is the potential for memory exhaustion during Borsh deserialization of variable-length fields where bounds checks occur after allocation rather than before. The unbounded finalization polling loop is a moderate concern mitigated by the expectation that callers use contexts with deadlines. No critical vulnerabilities were found.