feat: GnoConnect wallet interop, a selectable network, and failures the user can read - #19
Open
D4ryl00 wants to merge 17 commits into
Open
feat: GnoConnect wallet interop, a selectable network, and failures the user can read#19D4ryl00 wants to merge 17 commits into
D4ryl00 wants to merge 17 commits into
Conversation
Replace the legacy tosignin/tosign deep links with the GnoConnect contract: - Login: tosignin -> connect (land.gno.gnokey://connect). Display-level sign-in; the callback returns the address. Drop client_name. - Signing: tosign -> tx launch link in sign-only mode (broadcast=false). boards2 no longer builds the txJson; it passes path/func/args (positional args as a back-compat alias) + signer (pins the connected account) and gnokey builds, reviews, signs, and returns the signed tx as base64 signedtx, which we broadcast. - Callback security: an opaque single-use state token is issued with each launch and validated on return (LinkingProvider), rejecting forged/ unsolicited callbacks on the public callback scheme; non-success statuses are ignored. setLinkingData base64-decodes signedtx to amino-JSON, so the consumer screens drop their now-redundant decodeURIComponent. - Login gates on the address only (connect returns no remote URL; boards2 knows its own network). Not build-verified (no RN/Expo toolchain here); tsc adds no new errors over the pre-existing missing-@berty/gnonative-ui module errors.
Four issues found verifying the GnoConnect migration: - **A callback without `state` was accepted.** The check only ran when the parameter was present, so a forged `land.gno.boards2:/…?status=success& address=…` bypassed it entirely by omitting it — no token to guess. Every link we open (connect and tx alike) sends a state, so a callback without one cannot be a reply to us; it is now rejected. - **Tokens lived in a module-level Set,** which is empty after a cold start. The round trip leaves this app for the wallet and the OS may evict us while backgrounded, so a legitimate callback would be dropped on exactly the restart it triggers — intermittent, and it would look like the wallet's fault. Tokens are now persisted (expo-file-system, already a dependency) with a 15-minute TTL and single-use consumption. - **Positional `args=`.** The standard asks producers to emit named `arg.<name>`; positional is a back-compat alias that binds by position, so a divergence from the realm's declaration order silently binds values to the wrong parameters. Named args are now emitted for the functions whose signature we have, taken from the realm sources rather than guessed. `AddReaction` and `RepostThread` keep positional: neither is an exported function of gno.land/r/gnoland/boards2/v1 (there is only a renderRepostThread *render* route), so there is no signature to name them from — those calls look broken independently of this migration. - **`cancelled`/`error` were logged and dropped,** leaving the screen that opened the wallet waiting on a request the user had declined. The status is now recorded in the slice and exposed via `selectLinkingFailure`. Typecheck is clean (0 errors) once @berty/gnonative-ui is installed — it was simply missing from node_modules, which is what the "not build-verified" note on the migration commit was really reporting.
Login threw on every GnoConnect sign-in. The migration relaxed the gate in
app/index.tsx to require only the address, but the `loggedIn` thunk it
dispatches still demanded `remoteURL` from linking state — and `connect`
deliberately returns no RPC endpoint, because a dapp must never take one from
a callback it cannot authenticate. Nothing else set it, so the thunk threw
"No bech32 address, chainId or remoteURL found for login" every time.
Complete the change the migration started ("boards2 knows its own network"):
REMOTE/CHAIN_ID constants, applied when gnonative is initialised, and `loggedIn`
now requires only the address.
`connect` does report which chain the wallet is on, and that is worth checking:
if it differs from ours the wallet would sign for one network while we broadcast
to another, producing a signature that cannot land. Login now fails loudly on
that mismatch instead.
NOTE: REMOTE/CHAIN_ID default to the local gnodev (127.0.0.1:26657, chain
`dev`) because the app had no network of its own to inherit — it had always
taken one from the wallet. Point them elsewhere for a public chain.
`broadcastTxCommit` returns a stream — `Promise<AsyncIterable<…>>` — so
awaiting it only hands back the iterator. Nothing iterated it, so the call was
never driven and the signed transaction never left the device. It logged
`broadcasted tx: {}` and looked like a success while the chain saw nothing.
Iterate the stream and report the height/hash, and fail loudly if it yields no
response rather than reporting a silent success.
Only reachable now: under the old `tosign` flow the wallet broadcast, so this
path did nothing until sign-only made boards2 the broadcaster.
Broadcasting the wallet's signed transaction failed with ErrInvalidPubKey(#207) and nothing reached the chain. gnokey-mobile signs with a *session* key: the signature carries `session_addr` and the session's pubkey while `caller` is the master. gnonative 4.8.0 has no notion of session accounts anywhere in the package, so it re-encoded the transaction without that field, leaving one that claims `caller = master` but is signed by the session key — which the chain correctly rejects. 5.0.1 vendors a gno with session accounts (querySessionAccount, createSession, revokeSession and friends). All seven methods this app uses are unchanged, and tsc is clean across the major bump. Worth noting for the standard: sign-only (`broadcast=false`) quietly assumes the producer can serialise whatever signature scheme the wallet used. A client can accept a `signedtx` it is unable to broadcast, and the failure only shows up at the very last step.
The avatar comes from a separate realm (r/demo/profile) that may be absent, reloading, or erroring on a given network. In getUser the fetch was uncaught, so a profile failure threw all the way up and collapsed the whole board list to "No Boards yet." Wrap it and fall back to the default avatar, so a cosmetic miss never takes down the list. Also downgrade loadBech32AvatarFromChain's already-caught failure from console.error to console.warn: falling back to the default is expected degradation, not an error worth a red LogBox alarm.
Follows the GnoConnect standard settling on two things. Sign-only is a host, not a param: send `signtx://` and drop `broadcast=false`. An unknown param is silently ignored, so a wallet predating sign-only would have broadcast a transaction we asked it only to sign; an unknown host is declined with `unsupported_host` instead. The failure callback carries `code`, an enumerated machine value (`no_signer`, `signer_unavailable`, `tx_failed`, `network_declined`), where it used to carry `message`. Renamed the stored field to match, since the point of the rename is that it is branched on, not displayed.
The GnoConnect spec has `connect` take rpc/chainid so the wallet can offer to
switch networks before answering. boards2 sent only callback and state, so
gnokey had nothing to switch to and answered from whatever network it happened
to be on. The mismatch then surfaced at `loggedIn`, as a refusal the user had no
way to act on from the sign-in screen.
boards2 is the producer and owns the network, so name it. `signtx` already did;
only `connect` was missing.
Also log the launch link as a string: Hermes prints a URL object as `{}`, which
made the outgoing link impossible to inspect.
Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
The RPC endpoint and chain ID were hardcoded constants, so the only way to point the app at another chain was to edit and rebuild. That is wrong for anyone running a custom node, and it made the Android emulator unusable out of the box: loopback is relative to the device, so 127.0.0.1 reached the emulator itself rather than the gnodev on the host, which is at 10.0.2.2. A network is now one unit — endpoint plus the chain ID that endpoint serves — picked whole from a screen, never as two free-text fields, so a chain ID cannot be paired with an endpoint serving a different chain. Topaz is the default; Local gnodev carries the platform-aware loopback; Custom covers the rest. The selection is persisted with the same expo-file-system approach already used for callback state tokens, and presets are restored by id so a preset whose endpoint later changes reaches someone who chose it long ago. Switching resets the whole store. The connected address came from a `connect` against the old chain and every cached board is that chain's state, so carrying any of it over renders one chain's data under another chain's identity — wrong content rather than an error. The reset is applied where each slice is registered, so a slice added later is cleared too. The picker is reachable from the sign-in screen, not only from Settings: since `connect` names the network the wallet should switch to, it has to be selectable before signing in, and the profile screen is behind the auth guard. `network` is therefore allowed while signed out. Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
Failures were handled three different half-finished ways and none of them reached the user. `boardsSlice`, `threadsSlice` and `boardsCreateSlice` each stored an `error` no screen read. `linkingSlice` stored a `failure` whose own comment says a screen should explain it — none did, and `selectLinkingFailure` was exported and unused. Five screens caught broadcast rejections into `console.error`. In a release build there is no LogBox either, so approving a transaction in the wallet and returning to nothing happening was the whole user-visible behaviour. Route all of it through one channel. `errorReporter` middleware catches both shapes — a rejected thunk, and a non-success wallet callback, which is a plain action because the wallet did answer, just not `success` — and feeds a queue that one snackbar renders. No screen was changed: that is what makes it uniform rather than five copies of the same code. The snackbar is mounted at the app root, not per screen, because these failures arrive after a round trip through the wallet, by which point the screen that started the action may be gone or the app relaunched. It does not block: these are reports, not decisions. `describeError` turns anything into one sentence. Raw text is not usable — a dead RPC produced `[unknown] invoke bridge method error: unknown: … dial tcp …: connect: connection refused`, which is a developer's reason, not a user's. It prefers gnonative's `ErrCode`, which travels inside the message text as `ErrOutOfGas(#211)` and so survives the serialisation Redux Toolkit applies to a rejection before any middleware sees it. The deny list is deliberately a deny list: the problem was failures reaching nobody, so a thunk added later is reported by default and silence is what has to be justified. patches/ holds the version of describe-error.ts for once gnolang/gnonative#231 is released, verified end to end against a local build of it: the map shrinks to the entries naming a boards2 screen and the connectivity regex goes away. Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
Three things wrong in the same band at the top of the sign-in screen, measured from screenshots rather than judged by eye. `HomeLayout` renders its header outside the view that paints the page and gives it no background, so the bare window colour showed through: #F2F2F2 in the header against #FDFDFD below, a visible seam. The header now paints `theme.colors.background` itself, so it stays tied to the theme rather than being a hardcoded copy of it. The network icon rendered 55px, about 21dp at 420dpi. The touch target was already fine at 48dp through hitSlop, but as the only thing in the header, against a 120dp logo, it read small. Now 28dp. The icon set keeps its 24dp default; only this call site asks for more. Painting the header lighter then exposed a pre-existing problem: `expo-status-bar` is a dependency that was never rendered anywhere, and with userInterfaceStyle automatic the system chose light status bar icons, invisible against every screen in this light-only app. The darkest pixel in that band was exactly the background colour, before and after. `StatusBar style="dark"` at the root fixes it for every screen, not just this one. Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
It held the version of describe-error.ts to apply once gnolang/gnonative#231 released. That PR is closed, superseded by #232, which sends the message with the code instead of shipping a TypeScript map — so the file describes an approach that no longer exists. describe-error.ts already goes further than the patch did: it reads the code and the wording from ErrDetails and keeps only the two overrides that name a screen of this app. Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
gnonative 5.1 sends an ErrCode and default wording with every failure, as
ErrDetails on the error itself. This app was matching `ErrOutOfGas(#211)` out of
the message and carrying a table of thirty sentences to go with it — wording it
had to invent, and which said different things from every other gno app.
The detail only exists while the error object does: Redux Toolkit flattens a
rejection to `{ name, message, stack, code }` before any reducer or middleware
runs, so by the time errorReporter sees it the ConnectError is gone. Serialising
is the last point it can be read, hence createAppAsyncThunk — a createAsyncThunk
with a serializeError that keeps the code (as a string, the only shape RTK
preserves) and gnonative's wording. Every thunk uses it, including ones that
throw while iterating a stream, which a wrapper around the client would miss.
describeError shrinks to what only this app can say: two codes that name its
network settings screen. Everything else shows the library's sentence, so
boards2 says what every gno app says for the same failure. A realm's own
rejection is attributed — "The chain replied: …" — rather than spoken in the
app's voice, since a realm chooses those words.
Failures that reach here uncoded still have the bridge's framing on them
("[unknown] stream receive error: unknown: …"), so the stripping stays, now
covering all three bridge contexts and peeling the layers as they nest.
… it fails Two ways a creation screen ended up lying about what had happened. **It never stopped waiting.** Declining in the wallet left the submit button on "Loading" for good. The answer arrives as a deep link into a screen that never lost focus, so the `navigation` 'focus' listener that clears the flag does not fire, and no thunk rejects because nothing was signed. useWalletFailure watches the failure the wallet callback already puts in the store, which is the only signal that the round trip ended. **It went back on failure.** `await dispatch(broadcastTxCommit(…))` resolves even when the thunk rejects, so `router.back()` ran regardless and the catch beside it was dead code: a thread that was never created closed its screen as if it had been, taking what the user typed with it. `.unwrap()` makes the rejection reach the catch, which now keeps the screen and its filled-in form so they can submit again — the snackbar already says why.
The comments across this branch argue their case at a length that makes them harder to read than the code they sit above: several restate history that belongs in a commit message, some spend a paragraph on a decision a sentence settles, and a few explain the same thing twice. Keep the facts a reader cannot recover from the code — that a callback scheme is public, so `state` is required; that presets restore by id so a moved endpoint reaches whoever chose it; that a snackbar sits at the root because the screen that started the action may be gone — and drop the narration around them. No behaviour changes.
signedtx is base64 amino-binary and the standard requires a producer treat it as opaque and broadcast it unmodified. We were base64-decoding it to a UTF-8 JSON string and handing that to gnonative.broadcastTxCommit, whose parameter is signedTxJson — precisely the decode-and-re-encode the rule exists to prevent. A session key or multisig signature carries fields a generic client drops on that round trip, producing a well-formed-looking but invalid transaction that fails at the last step and looks like the wallet's fault. So the blob is stored exactly as received and POSTed to the chain's RPC as the single parameter of broadcast_tx_commit, which is what it already is. We never need to understand the signature at all. That path also checks the result: a node can accept a transaction that then fails on-chain, and check_tx and deliver_tx say which. The stream-iteration version only distinguished "delivered" from "not sent", so an on-chain failure read as success. txJsonSigned is renamed signedTx across the screens that read it — the old name described the amino-JSON it no longer holds.
Two defects, one visible and one that hid it. **Every failure read "Internal error".** tm2 answers a JSON-RPC error with the generic -32603 message and puts the actual cause in `error.data` — "Could not find tx result for hash …", and so on. The error path read `message`, so every distinct failure came out as the same three words and the diagnosis was thrown away at the point it was needed. It now reports `data`, falling back to `message`. **`broadcast_tx_commit` fails for transactions that land.** It holds the request open until the transaction is in a block, and when that wait fails — a timeout, a problem with the event subscription it depends on — the RPC returns an error for a transaction the node already accepted and goes on to commit. Reporting failure for a transaction that succeeded is the worst answer available, and it is what the user saw. Broadcast is now `broadcast_tx_sync`, which returns once the node has accepted the transaction into its mempool — the part the node can be definitive about, and what Adena uses against the same endpoints. Acceptance is not landing, so the hash is then polled through `tx` until it appears in a block. That is the standard's own guidance — a broadcast result is a hint, and a producer should confirm on its own RPC — and it restores the "it is on chain" signal that `_commit` was really being used for, without depending on the machinery that was failing. An accepted transaction that never appears is reported as exactly that, rather than as a failure: it may still land.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves boards2's wallet interop from the old
gnokey-mobile://tosignlinks togno's GnoConnect standard for external wallets
(
docs/resources/gnoconnect.md, launch links in gnolang/gno#5970), and makesevery failure along the way something the user can read.
15 commits, in three groups:
GnoConnect (
ea585bb…42a6d78) —connectfor sign-in,signtxforsigning, with boards2 broadcasting. It names the network it expects on every
link, requires and validates the callback
statetoken (a callback scheme ispublic — anything installed can open ours), and reads the wallet's enumerated
coderather than parsing prose.The network is the app's (
d231ed2) — boards2 is the GnoConnect producer,so it owns the rpc/chain-id it works against and tells the wallet. That needs a
screen to pick one, reachable before sign-in, and a store reset on switch: cached
boards and threads belong to the chain they came from.
Failures reach the user (
623f191,9e75580,3580481) — every rejectedthunk and non-success wallet callback now surfaces in one snackbar, worded from
the
ErrCodegnonative 5.1 sends with the error rather than from its messagetext. A declined request stops the spinner; a failed broadcast keeps the form so
it can be retried.
Where to look
redux/features/linkingSlice.tssrc/utils/callback-state.tsstateis required, and persistedredux/utils/async-thunk.tssrc/utils/describe-error.tsredux/middleware/error-reporter.tsVerification
tsc,eslintandprettierclean. Exercised on an Android emulator and an iOSsimulator against a local gnodev, with gnokey-mobile as the wallet: sign-in,
create board/thread/reply, network switch, and the refusal paths — declining in
the wallet, a realm rejecting the call, and an unreachable node.
Needs
@gnolang/gnonative≥ 5.1.0 for theErrDetailsthe error wording comesfrom.