fix(nostr): do not pick relays around (0, 0) for an undecodable geohash - #892
fix(nostr): do not pick relays around (0, 0) for an undecodable geohash#892Chessing234 wants to merge 2 commits into
Conversation
Geohash.decodeToBounds maps an empty or malformed string onto the degenerate box at (0, 0), and decodeToCenter therefore returns (0, 0) — which is a real place in the Gulf of Guinea, so a caller cannot tell "could not decode" from "the user picked the origin". Add isValid plus decodeToCenterOrNull / decodeToBoundsOrNull, and say in the doc comments which one to reach for. The existing decoders keep their behaviour, since the map view relies on getting a box back.
closestRelaysForGeohash wraps the decode in a try/catch and returns an empty list on failure, but decodeToCenter never throws: it hands back (0, 0), so an empty or malformed geohash quietly selects the relays nearest the Gulf of Guinea and connects to them. Use the nullable decoder, so the empty-list path the code already intended is the one that runs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eb49e4dae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (e: Exception) { | ||
| // An empty or malformed geohash decodes to (0, 0), which would pick the | ||
| // relays closest to the Gulf of Guinea rather than declining to choose. | ||
| val center = com.bitchat.android.geohash.Geohash.decodeToCenterOrNull(geohash) |
There was a problem hiding this comment.
Because this changes which Nostr relays are selected for geohash traffic, it affects routing/messaging; the reviewed commit only reports the geohash JVM unit test, but the repo requires physical-device Mesh Lab validation for routing or messaging changes. Please add the Mesh Lab result before merging so this relay-selection path is covered end-to-end.
AGENTS.md reference: AGENTS.md:L100-L103
Useful? React with 👍 / 👎.
|
on the mesh lab ask — i don't have a second android handset to run the two-device routing session, so i can't produce that result honestly, and i'd rather say so than paste a checklist i didn't run. what this change actually moves: RelayDirectory.closestRelaysForGeohash previously decoded an invalid geohash to (0, 0) and returned the relays nearest that point in the gulf of guinea. it now returns none for input that doesn't decode, and is unchanged for every geohash that does — decodeToCenterOrNull returns the same center as the old decoder on all valid input, which the added unit tests cover over the full charset. so the routing behaviour for real geohash channels is byte-identical; the only difference is on input that could never have selected the right relays anyway. happy to hold it for a maintainer with a device pair if you'd still like the mesh lab run on top of that. |
1 participant
RelayDirectory.closestRelaysForGeohashguards the decode:but
decodeToCenternever throws. An empty or malformed geohash decodes to the degenerate box at (0, 0), so instead of declining, the app sorts every relay by distance to the Gulf of Guinea and connects to the five closest —NostrRelayManager.ensureGeohashRelaysConnectedthen caches that set for the geohash.The underlying problem is that the decoder cannot say "this names no cell": (0, 0) is a real place, so
decodeToCenter("")anddecodeToCenter("s0000")are indistinguishable to a caller.Geohash.isValid,decodeToCenterOrNullanddecodeToBoundsOrNull, leaving the existing decoders alone (the globe view wants a box back regardless);RelayDirectory, so the empty-list path the code already intended is the one that runs../gradlew :app:testDebugUnitTest --tests "com.bitchat.android.geohash.*"passes locally, including the five new cases inGeohashValidityTest(tests="5" failures="0"), which cover: base32 accepted in either case,a/i/l/oand punctuation rejected, the nullable decoders returning null exactly where the plain ones return the origin, agreement on real cells, ands0000— the cell that actually contains (0, 0) — still decoding.