Skip to content

geolocation: add result destination for LocationOffsets#3480

Merged
ben-dz merged 4 commits into
mainfrom
bdz/geolocation-alt-offset-dest
Apr 14, 2026
Merged

geolocation: add result destination for LocationOffsets#3480
ben-dz merged 4 commits into
mainfrom
bdz/geolocation-alt-offset-dest

Conversation

@ben-dz
Copy link
Copy Markdown
Contributor

@ben-dz ben-dz commented Apr 7, 2026

Resolves: #3468

Summary of Changes

  • Rust SDK SetResultDestinationCommand that builds the onchain transaction with user PDA + probe account metas
  • CLI user set-result-destination --destination <host:port> (or --clear) with client-side validation of IP routability and domain format before transaction submission
  • Display result_destination in user get output (shows "none" when unset)
  • Go SDK ResultDestination string field on GeolocationUser with backwards-compatible deserialization (old accounts default to "")
  • Subcommand registration in the geolocation CLI binary

Stacked on #3501 which adds the onchain SetResultDestination instruction and result_destination field.

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 3 +425 / -8 +417
Scaffolding 6 +13 / -0 +13
Tests 1 +49 / -2 +47

Core logic is the CLI command (290 lines including validation + tests), Rust SDK command (118 lines), and Go SDK deserialization. Scaffolding is trait wiring and subcommand registration.

Key files (click to expand)
  • smartcontract/cli/src/geolocation/user/set_result_destination.rs — CLI command with clap args, client-side host:port validation (IP routability + domain format per RFC 1035), mock tests for set/clear/invalid inputs
  • smartcontract/sdk/rs/src/geolocation/geolocation_user/set_result_destination.rs — Rust SDK command: validates code, derives user PDA, passes probe account metas to execute_transaction
  • sdk/geolocation/go/state.goResultDestination string field, serialize/deserialize with try-decode-default for backwards compat
  • sdk/geolocation/go/state_test.go — round-trip and backwards-compat tests (truncated data defaults to empty)
  • smartcontract/cli/src/geoclicommand.rsset_result_destination trait method + impl

Testing Verification

  • CLI mock tests: set destination, clear, missing destination flag, 9 invalid destination cases (private IPs, bad ports, malformed domains)
  • Go SDK: round-trip test with non-empty destination, empty-targets test, backwards-compat test (truncated data)
  • All tests pass: 289 CLI, 165 SDK, 82 geolocation, Go SDK green

@ben-dz ben-dz added this to the Geo Location milestone Apr 7, 2026
@ben-dz ben-dz marked this pull request as ready for review April 8, 2026 13:00
@ben-dz ben-dz requested review from karl-dz and nikw9944 and removed request for karl-dz and nikw9944 April 8, 2026 13:02
@ben-dz ben-dz marked this pull request as draft April 8, 2026 13:13
@ben-dz ben-dz marked this pull request as ready for review April 8, 2026 15:17
@ben-dz ben-dz requested review from juan-malbeclabs, karl-dz and nikw9944 and removed request for juan-malbeclabs April 8, 2026 15:17
@ben-dz ben-dz force-pushed the bdz/geolocation-alt-offset-dest branch 3 times, most recently from a822d1c to baf7ff2 Compare April 9, 2026 17:19
@ben-dz ben-dz changed the base branch from main to bdz/geolocation-alt-offset-onchain-only April 9, 2026 17:22
@ben-dz ben-dz force-pushed the bdz/geolocation-alt-offset-dest branch from baf7ff2 to a0d9f8e Compare April 9, 2026 17:31
@ben-dz ben-dz force-pushed the bdz/geolocation-alt-offset-onchain-only branch from 84e4f3b to 569f815 Compare April 9, 2026 17:31
@ben-dz ben-dz requested a review from juan-malbeclabs April 10, 2026 12:48
@ben-dz ben-dz marked this pull request as draft April 10, 2026 12:48
Base automatically changed from bdz/geolocation-alt-offset-onchain-only to main April 10, 2026 20:16
Rust SDK command, CLI with client-side destination validation, display in
user get, Go SDK deserialization with backwards compatibility.
@ben-dz ben-dz force-pushed the bdz/geolocation-alt-offset-dest branch from a0d9f8e to cd0e330 Compare April 10, 2026 20:38
@ben-dz ben-dz marked this pull request as ready for review April 10, 2026 20:39
Copy link
Copy Markdown
Contributor

@juan-malbeclabs juan-malbeclabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, review Account Order

@ben-dz ben-dz force-pushed the bdz/geolocation-alt-offset-dest branch from 2dfb623 to 5b74c81 Compare April 14, 2026 13:50
Copy link
Copy Markdown
Contributor

@juan-malbeclabs juan-malbeclabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.Two more comments: the first is to prevent the changelog from ending up incorrect, and the second is purely semantic

Comment thread CHANGELOG.md Outdated
@ben-dz ben-dz force-pushed the bdz/geolocation-alt-offset-dest branch from 28bf773 to 0073f5d Compare April 14, 2026 15:01
@ben-dz ben-dz enabled auto-merge (squash) April 14, 2026 15:20
@ben-dz ben-dz merged commit f95ae4c into main Apr 14, 2026
33 checks passed
@ben-dz ben-dz deleted the bdz/geolocation-alt-offset-dest branch April 14, 2026 15:38
ben-dz added a commit that referenced this pull request Apr 17, 2026
## Summary

- Route outbound LocationOffsets to the user's `ResultDestination` when
set on `GeolocationUser`, instead of sending to the measurement target
address
- Skip ICMP targets that have no result destination (nothing is
listening at the measurement IP)
- Add DNS resolution with caching (5min TTL) for domain-based result
destinations
- Validate delivery addresses against public-IP scope checks to prevent
DNS rebinding/SSRF attacks
- Validate `ResultDestination` format at discovery time to surface
errors early

## Details

The `ResultDestination` field was added to `GeolocationUser` in
#3480/#3501. This PR implements the geoprobe-agent side: reading the
field during target discovery, carrying it through as `DeliveryAddrs` on
`TargetUpdate`/`ICMPTargetUpdate`, and routing offsets to the specified
address in `sendCompositeOffsets`.

Key design decisions:
- Delivery addresses are carried as a separate `map[ProbeAddress]string`
rather than modifying `ProbeAddress` itself, to preserve Pinger map key
semantics
- `DNSCache.Resolve()` validates resolved IPs against `ValidateScope()`
to prevent DNS rebinding
- Change detection for delivery addresses triggers target updates even
when the target list is unchanged

Depends on: #3468

## Testing Verification

- Unit tests for outbound target with result destination → delivery
override populated
- Unit tests for user without result destination → no delivery overrides
- Unit tests for ICMP target with result destination → delivery override
in ICMP update
- Unit tests for mixed users (with/without result destination)
- Unit tests for domain-based result destination passthrough
- Unit tests for delivery address change detection → triggers update
- Unit tests for invalid result destination format → ignored with
warning
- DNS cache: IP passthrough, domain resolution and caching, TTL expiry,
concurrent access
- DNS cache security: private IP rejection, DNS rebinding to private IP
rejected

Fixes #3469
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.

geolocation: add result destination to GeolocationUser (onchain + CLI + SDK)

2 participants