Repository navigation
NEAR cold signing: review and sign NEAR transactions with the ML-DSA-65 key - #677
Conversation
Adds api/near.rs: a borsh TransactionV0 decoder exposed to Dart as a flat NearTransaction/NearAction model, SHA-256 transaction hashing, pure FIPS 204 ML-DSA-65 signing with an empty context (what the NEAR runtime verifies), signature verification, and the ml-dsa-65:/ml-dsa-65-hash: text forms of a key. Regenerated flutter_rust_bridge bindings; U8Array32 moves to the shared lib.dart because two modules now use it. KeypairExtensions gains signNear with hedged entropy.
NearSigningRequest is envelope version 2 of the ur:quantus-sign-request
payload: {v:2, chain:'near', network, payload:0x<borsh tx>} with a strict
key set and the existing 8 KiB cap. AnySigningRequest.decode dispatches
on v so a v1 wallet refuses v2 by version as before. NearPublicKeyExport
is the self-describing JSON a cold wallet shows when publishing an
ML-DSA-65 key for NEAR.
The scanner routes a v2 request to SignNearTransactionScreen, which finds the ML-DSA-65 account holding the key the transaction names, shows the network, signer, receiver and every action in full (key, code and account changes in warning colour), warns when account names do not belong to the labelled network, and on approval emits signature ‖ public key as the usual animated UR QR. ShowKeyScreen offers an ML-DSA-65 account's key in NEAR's form as an animated QR with its on-chain handle. The signature QR view is shared between the Quantus and NEAR screens. Debug builds get a NEAR sample catalogue built by a minimal borsh writer that matches near-cli-rs byte for byte.
decode_near_transaction checks signer, receiver, beneficiary and function-call-key receiver ids against NEAR's rules, which also keeps every account id shown to a signer plain ASCII.
…ay-safe Control, bidi and zero-width format characters in a prepared transaction's strings could reorder or hide what the review shows; such text falls back to its bytes.
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT-6 Sol
Verdict: Request changes. The NEAR signing screen can present an untrusted network label as if it were validated, while silently skipping its account-name mismatch check.
P1 — Reject noncanonical network labels before review. NearSigningRequest.decode accepts every nonempty network string (quantus_sdk/lib/src/models/signing_request.dart:111-116), but NearDisplay.networkMismatch checks only exact mainnet and testnet and returns no warning for all other values (cold-wallet-app/lib/services/near_display.dart:89-95). For example, network: "testnet " displays as NEAR · TESTNET with an unobtrusive trailing space (sign_near_transaction_screen.dart:252), yet suppresses the warning for a transaction naming alice.near and bob.near. A signer can therefore approve a mainnet transfer while the review appears to identify it as testnet. Validate the envelope against explicit supported network names before displaying it, and add a regression case for a lookalike label with mainnet account names.
Validation at head 395ba980: cargo test --locked --lib api::near::tests --quiet (8 passed); focused SDK Flutter tests (18 passed); focused cold wallet Flutter tests (16 passed); dart format --output=none --set-exit-if-changed --line-length=120 on changed Dart files and git diff --check passed. GitHub's Analyze check is green. No other blocking findings found in the reviewed diff.
The label is what the signer is told and what selects the account-name check, so a lookalike such as "testnet " could read as testnet while disabling the testnet check. NearSigningRequest.decode now refuses anything but the supported names, and NearDisplay.networkMismatch treats any other label as a programming error rather than a pass.
|
Addressed in dd21c97. P1 — network labels. The CLI side mirrors this in Quantus-Network/quantus-cli#174 ( SDK models 31 pass, cold-wallet-app 316 pass, |
Two WASM blobs of the same length produced the same review; the digest is how NEAR identifies code, so a signer can compare it with the artifact they meant to deploy.
n13
left a comment
There was a problem hiding this comment.
Verdict: Request changes.
The core integration makes sense and matches the merged quantus-cli PR #174: strict v2 NEAR requests, Borsh TransactionV0 decoding, matching the transaction public key to a held ML-DSA-65 account, pure ML-DSA-65 over SHA-256 with an empty context, and the 5261-byte signature-plus-public-key response. I exercised the full app path through the regenerated native bridge with a public test mnemonic and verified the resulting signature and QR roundtrip. The noncanonical-network-label issue from the earlier review is fixed at this head.
There are two correctness findings and two duplication findings below: JSON argument display changes numeric values; the dangerous-action banner identifies the signer rather than the affected receiver; the key-export screen copies the signature QR controls; and the NEAR refusal layout copies the existing Quantus refusal layout. The new signing and QR state also remains in widgets despite code-rules.mdc requiring provider-owned state; the shared components/controllers should follow that rule during extraction.
The NEAR key UI is Show Key -> Show NEAR public key for an ML-DSA-65 account only. It exports the same public key in NEAR's text form, displays an animated QR, the ml-dsa-65-hash on-chain handle, the Quantus address, and the full key. A sample with default settings produced 10 frames at 10 FPS. This does not create or register a NEAR account. Public-key QR import into the hot wallet/CLI is explicitly deferred, so the bootstrap flow remains incomplete; the instructions should reflect that limitation, and the roughly 2700-character full key currently has neither selection nor a copy control.
Validation at f7756c9: 8 Rust NEAR tests, 19 SDK request/export tests, 18 NEAR display/review tests, 2 existing scanner/Show Key tests, and 3 temporary native app integration/render tests passed (50 total). Three temporary regression probes failed as expected, reproducing numeric precision loss (integer and decimal) and the wrong account in the control-change warning. GitHub Analyze is green. No production source changes were made.
| final decoded = json.decode(utf8.decode(args, allowMalformed: false)); | ||
| final pretty = const JsonEncoder.withIndent(' ').convert(decoded); | ||
| return isDisplaySafe(pretty, allowNewlines: true) ? pretty : '0x${hex.encode(args)}'; |
There was a problem hiding this comment.
[P1] Preserve the exact numeric arguments during review
Parsing the signed argument bytes into Dart numbers and encoding them again changes what the signer sees. I reproduced {"amount":18446744073709551617} displaying as {"amount":18446744073709552000.0}, and 0.123456789012345678901234 displaying as 0.12345678901234568. The original bytes are still signed, so the reviewed amount or limit can differ from the value supplied to the contract. Pretty-print with a formatter that preserves numeric tokens exactly, or show the display-safe original JSON without converting its numbers; add regression cases for large integers and long decimals.
There was a problem hiding this comment.
Fixed in e5f2074. argsText now validates the bytes as JSON and then re-indents the original text with a token-copying walker (NearDisplay.reindentJson): only whitespace outside strings changes, numbers are never decoded. Regression cases: 18446744073709551617, 0.123456789012345678901234, 1E+400, -0, plus nesting/empty containers/escaped quotes.
| if (dangerous) | ||
| _warningBanner( | ||
| context, | ||
| title: 'This changes control of ${tx.signerId}', |
There was a problem hiding this comment.
[P2] Name the receiver in the account-control warning
These actions affect receiver_id, which can differ from signer_id. For a valid account-creation batch signed by alice.testnet with receiver vault.alice.testnet and CreateAccount + AddKey, this banner says "This changes control of alice.testnet" even though the new key controls vault.alice.testnet. I reproduced this in a widget test. Use the affected receiver in the banner so the prominent safety message agrees with the action details.
There was a problem hiding this comment.
Fixed in cf1bbef: the banner now names tx.receiverId. Widget test added for a CreateAccount + AddKey batch signed by alice.testnet with receiver vault.alice.testnet.
| Future<void> _pauseAndTune() async { | ||
| setState(() => _qrPaused = true); | ||
| await BottomSheetContainer.show<void>( | ||
| context, | ||
| builder: (ctx) => const BottomSheetContainer(title: 'QR display options', child: QrTuningControls()), |
There was a problem hiding this comment.
[P2] Reuse one QR presentation component and controller
This screen duplicates SignatureQrView's fragment cache, pause/tuning-sheet flow, animation, frame/FPS/byte summary, and pause-button styling. Extract the QR body and its provider-owned state into a reusable component accepting the payload; keep the screen titles, details, and Done navigation in their respective screens. Sharing only AnimatedUrQr leaves the surrounding behavior and styling copied in two places, contrary to the requested DRY review and the repository's shared-component/state rules.
There was a problem hiding this comment.
Done in 305cfc5. UrQrPanel (components/ur_qr_panel.dart) renders any payload: frames, summary line, pause button, tuning sheet, animated note. Its state is provider-owned in providers/ur_qr_providers.dart: urQrFramesProvider (autoDispose family keyed by a content-equal UrPayload, derived from coldSettingsProvider.qrBytes) and qrPausedProvider (autoDispose Notifier). SignatureQrView and NearKeyExportScreen are now stateless and keep only their titles/details/Done navigation. Tests in test/ur_qr_panel_test.dart.
| Widget _errorView(BuildContext context, {required String title, required String message, required Widget detail}) { | ||
| final colors = context.colorsV3; | ||
| final text = context.themeTextV3; | ||
| return ScaffoldBase( | ||
| appBar: const V2AppBar(title: 'Sign NEAR Transaction'), |
There was a problem hiding this comment.
[P2] Share the existing signing-refusal layout
This _errorView copies the existing SignTransactionScreen._errorView: the warning icon, heading/message styling, detail container, "Nothing was signed" text, and home-navigation button are the same. Extract a shared refusal view parameterized by app-bar title, heading, message, and detail, then use it from both signing screens. The signature-success view is already shared in this PR; the refusal path should receive the same treatment instead of adding another copy.
There was a problem hiding this comment.
Done in 305cfc5: SigningRefusalView(appBarTitle, title, message, detail) in components/signing_refusal_view.dart, used by both SignTransactionScreen and SignNearTransactionScreen.
…bers A Dart num cannot hold every JSON number, so decode-then-encode showed 18446744073709551617 as 18446744073709552000.0. The JSON is now validated, then re-indented from its own text with every token copied through verbatim.
Key, code and account actions apply to receiver_id, which differs from the signer when a batch creates a sub-account.
…creens UrQrPanel renders any payload as an animated UR QR with the frame summary and pause/tuning control; its frames and paused flag live in providers (urQrFramesProvider, qrPausedProvider) rather than in widgets. SigningRefusalView is the one refusal layout for the Quantus and NEAR signing screens. The NEAR key export uses both, makes the full key selectable with a copy control, and says that the hot wallet does not yet read its QR.
|
All four findings addressed at 305cfc5 (replies inline). Also from the summary: the export screen's full key is now a cold-wallet-app: 324 tests pass (8 new), |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT-6 Sol
Verdict: Request changes. The network warning can misidentify valid NEAR accounts as belonging to the wrong network.
P2 — Make the network warning a suffix hint, not an account-validity claim. In cold-wallet-app/lib/services/near_display.dart:166-180, every named account that does not end in .near (mainnet) or .testnet (testnet) triggers “Network does not match” and “is not a [network] account name.” A mainnet transaction from the valid top-level account near to alice.near therefore gets a false warning; a valid 0s-prefixed deterministic account also gets one because _isNamed handles only the older implicit forms. NEAR account IDs themselves do not encode the network, so a matching suffix cannot verify the request's network label either. In this signing review, describe the check as a naming-convention hint, identify the label as supplied by the requesting wallet, and cover top-level and 0s account IDs in regression tests. NEAR's account-ID rules document both forms.
The previous blocking findings on exact network labels, numeric JSON display, the affected receiver, and duplicated QR/refusal views are addressed at this head. No other blocking findings found.
Validation at 305cfc5: cargo test --locked --lib api::near::tests --quiet (8 passed); focused quantus_sdk Flutter tests (19 passed); focused cold-wallet-app Flutter tests (25 passed); changed-Dart formatting and git diff --check passed. GitHub Analyze and dependency-cooldown checks are green.
…y claim NEAR account IDs do not encode a network. Only sub-accounts ending in the other network's customary top-level name (.testnet on mainnet, .near on testnet) are mentioned; top-level accounts, implicit (64 hex, 0x+40 hex) and deterministic (0s+40 hex) IDs, and other hierarchies are never flagged. The copy names the label as the requesting wallet's and says names do not prove a network.
|
Addressed in 71bc5fa. P2 — network warning as a suffix hint. Focused tests: 22 passed; |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT-6 Sol
Verdict: Request changes. The earlier network-label and network-hint findings are fixed at head 71bc5fa5, but account creation still misses the safety treatment applied to other account changes.
P2 — Warn for CreateAccount without a key action. NearDisplay.isDangerous returns false for createAccount (cold-wallet-app/lib/services/near_display.dart:148-151). A decoded CreateAccount + Transfer batch therefore gets no account-change banner, and both the overall heading and the CREATE ACCOUNT row use ordinary colors (sign_near_transaction_screen.dart:117-141,270-275). The signer is authorizing creation of the receiver account and a transfer to it, yet the review gives this account change none of the safety cues described for the feature. Highlight account creation with accurate wording that names the receiver, and add a widget regression for a batch with no AddKey.
Validation at 71bc5fa5: cargo test --locked --lib api::near::tests --quiet (8 passed); focused SDK Flutter tests (19 passed); focused cold-wallet Flutter tests (27 passed); formatting of the latest changed Dart files and git diff --check passed. Workspace bootstrap resolved both affected packages but stopped in unchanged mobile-app because its generated iOS SourcePackages directory is absent. GitHub Analyze is still running. No other blocking findings found.
CreateAccount brings a new account into existence under the signer's authority; it now counts as dangerous like the other account changes. The banner names the receiver as created when nothing else in the batch is dangerous, and as a control change otherwise.
|
Addressed in 935c525. P2 — warn for Focused tests: 24 passed; |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT-6 Sol
Verdict: Request changes. The review screen presents the request's unsigned network label as an established network, which can mislead a signer about where the transaction will execute.
P1 — Identify the network as an unverified claim on every signing review. NearSigningRequest.network is display-only and is not part of the signed transaction (quantus_sdk/lib/src/models/signing_request.dart:90-92; quantus_sdk/rust/src/api/near.rs:367-370). Yet sign_near_transaction_screen.dart:200-202 renders NEAR · TESTNET or NEAR · MAINNET without attribution. The suffix hint in near_display.dart:174-190 is silent for implicit and other network-neutral account IDs. A QR can therefore contain a mainnet transaction and a canonical testnet label; the review shows NEAR · TESTNET with no warning, then signs bytes that are valid for mainnet. Show prominently on every review that the network is supplied by the requesting wallet and cannot be verified from these transaction bytes, and cover a network-neutral account case in a widget test.
The earlier findings on canonical labels, exact JSON numbers, affected account names, shared QR/refusal views, naming hints, and account creation are addressed at head 935c5259. I found no other blocking issue in this diff.
Validation at 935c5259: cargo test --locked --lib api::near::tests --quiet (8 passed); focused SDK Flutter tests (19 passed); focused cold-wallet Flutter tests (29 passed); changed Dart formatting and git diff --check passed. GitHub Analyze and both dependency-cooldown checks are green.
…nverified claim The label is not part of the signed bytes and nothing in them names a network, so every review now shows UNVERIFIED beside it with a standing notice that the signature is valid wherever the signer account and nonce exist, and tells the signer which network to confirm on the hot wallet.
|
Addressed in 8390428. P1 — network label shown as established. The chip is no longer bare. Every review now shows Tests: new widget test Focused tests: 26 passed; |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT-6 Sol
Verdict: Approve. The NEAR signing path now identifies the network label as the requesting wallet's unverified claim on every review, including transactions with network-neutral account IDs. The earlier findings on network labels and hints, exact JSON numbers, receiver warnings, account creation, and shared QR/refusal views are addressed at head 83904289. I found no blocking issue in the reviewed diff.
Validation: cargo test --locked --lib api::near::tests --quiet (8 passed); focused SDK Flutter tests (19 passed); focused cold-wallet NEAR display/signing/QR tests (31 passed); existing scanner and Show Key tests (2 passed); changed-Dart formatting and git diff --check passed. The Rust test run emitted one deprecation warning in test code. GitHub Analyze was still running when this review was posted; both dependency-cooldown checks were green.
…t a NEAR key An ML-DSA-65 key is a Quantus key; NEAR is one system that registers it directly. The section, screen title, copy and copy-toast now say public key, with NEAR named only where its text form and on-chain handle apply.
|
One more commit after the approval, 02f59a6, copy-only on the key export: an ML-DSA-65 key is a Quantus key, not a NEAR key, so the Show Key section is now titled PUBLIC KEY, the export screen is |
Cold-wallet side of NEAR cold signing with the Quantus ML-DSA-65 key. Pairs with Quantus-Network/quantus-cli#174 (
near sign-cold/cold-sign-sim), which already speaks the envelope and response format implemented here.What a user gets
{v:1, kind:"near-public-key", address, near_public_key:"ml-dsa-65:…"}, plus theml-dsa-65-hash:handle NEAR lists on chain once the key is added.ur:quantus-sign-requestwhose envelope is{v:2, chain:"near", network, payload:"0x<borsh TransactionV0>"}opens Sign NEAR Transaction: network chip, signer, receiver, every action with all parameters (function-call args pretty-printed, gas in Tgas, exact NEAR amounts), key/code/account changes in warning colour with a "this changes control of …" banner, a warning when account names do not belong to the labelled network, and Signing key held by with the Quantus address + checkphrase. Refuses (with no Sign button) a non-ML-DSA-65 key, a key no account here holds, or a locked wallet.signing_contextis bypassed), emitted assignature ‖ public_key(3309 + 1952 bytes) in the usual animated QR.Changes
quantus_sdk/rust/src/api/near.rs(new): borshTransactionV0decoder → flatNearTransaction/NearAction(refuses account ids the chain would refuse, which also keeps every id shown plain ASCII),near_transaction_hash,sign_near_transaction(requires ML-DSA-65 and that the keypair is the key the transaction names),verify_near_signature,near_public_key_text,near_public_key_handle. 7 unit tests, including the near-cli-rs 0.30.1 golden vector (hashbe358ec9…).U8Array32moved tolib/src/rust/lib.dart(now shared by two modules). Note: codegen on this machine wrotestem: 'UNKNOWN'because its bundledcargo_metadata 0.14can't parse edition-2024 crates; I restoredrust_lib_quantus_walletby hand to matchmain.signing_request.dart:AnySigningRequest.decodedispatching onv;NearSigningRequest(v2, strict keys, 8 KiB cap). v1 behaviour unchanged.NearPublicKeyExportmodel.KeypairExtensions.signNear(hedged).SignNearTransactionScreen,NearKeyExportScreen,NearDisplay(pure display rules; method names and arguments fall back to hex if they contain control, bidi or zero-width characters — same rule as the CLI'srender_args),nearKeyOwnerProvider, scanner routing,ShowKeyScreenNEAR section, sharedSignatureQrView(extracted fromSignTransactionScreen, no behaviour change), debug NEAR catalogue with a minimal borsh writer.Tests
cargo test --lib near::8 pass; no new clippy warnings.quantus_sdk: 575 pass (--exclude-tags=native), nativeur_qr_frame_testpasses against a release build; a temporary native test (not committed) decoded the golden vector, signed it with a derived ML-DSA-65 key, verified, and confirmed a wrong key is refused.cold-wallet-app: 315 pass, including newnear_display_test(asserts the debug borsh writer reproduces the near-cli-rs transfer vector byte for byte) andsign_near_transaction_screen_test(7 widget tests over the review and refusal states).dart format --line-length=120,dart analyzeclean on both packages.Not in this PR
near-public-keyexport QR (step 5 of the plan).ft_transferargs (shown as raw JSON, as the signer can't know decimals offline).