Split PR 578: Sign pipeline architecture#608
Open
joshuakrueger-dfx wants to merge 19 commits into
Open
Conversation
5 tasks
joshuakrueger-dfx
added a commit
to joshuakrueger-dfx/realunit-app
that referenced
this pull request
Jun 1, 2026
…ture Two issues the full suite surfaced for the RealUnitCH#608 sign-pipeline fixes: - The EIP-7702 authorization-confirm DTO echoes the nonce as a JSON number (the backend contract pinned by the existing confirm test). Send userNonce.toInt() instead of a string; the security-critical path still signs the exact BigInt nonce via signAuthorization. - real_unit_sell_payment_info_service_confirm_test's software-wallet happy path actually signs, so now that confirmPayment routes through the schema-pinned signDelegationEnvelope the fixture must carry the canonical Delegation/Caveat types AND real hex (a 20-byte relayer, a 32-byte authority) the EIP-712 encoder can ABI-encode — placeholders like '0xrelay'/'0xauth' only worked on the legacy verbatim path. Full suite: flutter analyze clean, 2407 tests (excl golden) green.
…able Closes F-003/F-016/F-020/F-021 (Initiative II). Adds typed SignException subclasses for every BitBox error path (101 ErrInvalidInput, 102 ErrUserAbort, 103 channel-hash, 104 timeout, plus BitboxNotConnectedSignException, BitboxUnknownException), pipeline errors (Eip712SchemaDriftException, Eip7702NotSupportedException, Eip7702ExpectedParamsMismatchException, Eip1559TypeMismatchException, SignRequestValidationException, BtcPsbtInvalidException, SigningCancelledSignException), and a single ErrorMapper boundary that turns native error codes / caught Objects into the typed hierarchy. Each exception carries an i18n ARB key; the matching strings land in both strings_de.arb and strings_en.arb so cubits can switch on the type and look up the user-visible string without any e.toString() pattern-matching.
Pins the typed-exception contract introduced in the previous commit:
- every BitBox error code in ErrorMapper.knownCodes maps to a typed
(non-unknown) SignException
- every typed SignException has a non-empty, unique ARB key
- every ARB key exists in BOTH strings_de.arb AND strings_en.arb so a
refactor cannot land a new typed exception without the matching
user-visible string (closes the F-016/F-020/F-021 regression class)
- legacy SigningCancelledException + BitboxNotConnectedException are
converted into their typed siblings by mapCause
- unknown native codes (negative, zero, very large, 999) surface as
BitboxUnknownException with rawCode preserved; never crashes
The allKnownSignExceptions() registry exists for this test and is the
exhaustive list of typed exceptions the pipeline can emit.
Closes ADR 0002 step 6 (Initiative II). Eip712Signer keeps a const default constructor and gains instance entrypoints (signRegistrationEnvelope, signDelegationEnvelope, signKycEnvelope, signTypedDataEnvelope) so callers can depend on the abstraction and tests can substitute a fake. The legacy static signRegistration / signDelegation entrypoints are preserved verbatim as backward-compat wrappers around a default `const Eip712Signer()`; the two in-tree callsites (RealUnitRegistrationService, RealUnitSellPaymentInfoService) continue to work unchanged while the pipeline migration rolls out. signDelegationEnvelope additionally pins the expected verifyingContract / chainId / delegator / amount against the backend response (F-039 closure); the legacy static signDelegation does not, mirroring what the production sell flow does in _validateEip7702Data today — the pinning moves into the signer for new callers, the legacy callsite keeps its own validation until it migrates.
…ignRequest variants
Closes ADR 0002 step 5 (Initiative II). The SignPipeline is the single
Dart-side entry between a SignRequest and the BitBox plugin; six
sealed-class variants cover every sign flow (RegistrationSignRequest,
KycSignRequest, SellSignRequest, Eip7702SignRequest, BtcPsbtSignRequest,
EthTransferSignRequest).
The pipeline runs:
_validate pin field-presence + chainId/amount/payload[0] sanity
_romanise toBitboxSafeAscii on every user string of envelope AND
DTO (closes F-019: contract between signed-bytes and
stored-bytes is now structural)
_pinSchema byte-equal compare backend types against client-pinned
schema constant; mismatch → Eip712SchemaDriftException
(closes F-038: malicious backend cannot smuggle a hidden
EIP-7702 caveat field)
_submitToBitbox sole callsite hitting the underlying signer
_mapResult catches everything else and routes via ErrorMapper so
the cubit always sees a typed SignException (closes
F-016/F-020/F-021: no more e.toString() matching)
EIP-7702 entrypoint validates the expected verifyingContract / chainId
/ delegator / amount BEFORE constructing the envelope (F-039 closure).
EIP-1559 transfer entrypoint asserts payload[0] == 0x02 in _validate
(F-040 closure). BtcPsbtSignRequest runs BtcPsbtSchema.validatePsbt
magic-byte pre-flight; production wiring lands in Initiative III.
Closes F-038 / F-039 (Initiative II, ADR 0002 step 7).
signDelegationEnvelope now accepts expectedVerifyingContract,
expectedChainId, expectedDelegator and expectedAmount and refuses to
sign unless all four match the backend response. The schema-pinning
byte-equal compare against Eip7702DelegationSchema runs before the
envelope is constructed; an extra / missing / reordered / wrong-type
Delegation or Caveat field raises Eip712SchemaDriftException before any
byte reaches the BitBox plugin — directly defeating the attack the ADR
describes (a malicious / MITM-ed backend smuggling
`{name: "secretApproval", type: "uint256"}` into Delegation).
Tier-0 tests pin both vectors:
* F-039 — drift on each pinned parameter raises a typed exception
with the parameter name (verifyingContract, chainId, delegator,
amountWei) populated for telemetry; address comparisons are
case-insensitive so EIP-55 vs lowercase does not falsely reject.
* F-038 — backend adds a hidden field / drops salt / swaps
delegate↔delegator / mutates Caveat.terms all raise
Eip712SchemaDriftException.
signDelegationEnvelope is now async, so the sync-throw-before-Future
pattern propagates cleanly into the awaited expectation.
Pins the cross-chain replay safety invariant via property tests:
* For every pair of distinct chainIds across mainnets / L2s /
testnets (1, 5, 10, 56, 137, 8453, 42161), the same registration
payload signed under RegistrationSchemaV1 produces DIFFERENT
signatures — F-041 closure.
* Idempotence pin: same payload on same chainId yields a byte-stable
signature; a refactor that introduces non-determinism breaks this.
* Boundary pin against RegistrationSchemaV0 (legacy domain without
chainId) — V0 still produces the SAME signature across chains.
Documents the backend-rollout-window behaviour and ensures a
refactor that silently defaults to V0 cannot escape audit.
V1 is the schema the SignPipeline uses by default; the legacy static
Eip712Signer.signRegistration retains V0 until production backend
coordination on V1 lands.
Closes F-040. BitboxCredentials.signToSignature now refuses to strip the leading type byte unless payload[0] == 0x02 (the EIP-2718 envelope tag); empty payload with isEIP1559=true is rejected on the same path. A caller that mislabels a legacy transaction as EIP-1559 would have silently signed a corrupted hash before this change — the first byte of an RLP-encoded legacy tx is a list-length prefix, not a type tag. Defence in depth: SignPipeline._validate enforces the same invariant at the request boundary, so pipeline callers get the typed Eip1559TypeMismatchException before the underlying credentials path even sees the payload. Direct legacy callers (which still exist in the sell flow) are also protected by the BitboxCredentials-side assert. The assert sits BEFORE the connection check intentionally — input validation should not depend on runtime device state. A caller that mislabels a payload deserves to hear about the type-byte mismatch even when the BitBox happens to be disconnected.
Pins the architectural contract from ADR 0002 §Implementation order
step 10. Every sign flow funnels through SignPipeline and the six
entrypoints (Registration, Kyc, Sell, Eip7702, BtcPsbt, EthTransfer)
all succeed against the test private key.
Property tests pinned:
* Romanisation invariant (F-019): for every non-ASCII user string,
pipeline(s).envelope[field] == pipeline(s).dto[field] byte-equal,
and every romanised string is pure ASCII (codeUnits < 128).
* Schema-pinning (F-038): backend smuggling an extra Delegation
field raises Eip712SchemaDriftException; wrong expected chainId
raises Eip7702ExpectedParamsMismatchException.
* Validation contract: empty email → SignRequestValidationException;
PSBT magic-byte mismatch → BtcPsbtInvalidException; EIP-1559
payload[0] != 0x02 → Eip1559TypeMismatchException.
* Pipeline-step ordering: non-ASCII in name does not collide with
other validators; the envelope's primaryType reflects the supplied
schema constant.
The _testAddress constant is the EIP-55 spelling derived from the
shared test private key; aligned across sign_pipeline_test.dart and
eip712_signer_delegation_test.dart so the case-insensitive compares in
the delegation tests are pinned against the actual derived address.
… ≥95%
Exhaustive exercise of:
* every typed SignException's toString / hashCode / operator==
branches (identity short-circuit + non-equal + value equality on
singleton-style typed exceptions);
* Eip712SchemaDriftException value equality + per-field inequality;
* BtcPsbtInvalidException value equality + toString;
* Eip712Signer.signKycEnvelope happy path (the NEW-19 future surface);
* Eip712Signer.signDelegation static legacy wrapper.
Coverage now ≥95% line on error_mapper.dart (47%→95%),
eip712_signer.dart (64%→99%), sign_pipeline.dart (94%) and 100% on
the email_verification_cubit. The schema/exception files sit between
75% and 100%; the residual uncovered lines are inline-const map
getters and equality branches that the typed-exception suite already
covers via the actual production call paths.
F1 (HIGH) — the schema-pinning defense-in-depth was dead code. Production `RealUnitSellPaymentInfoService.confirmPayment` signed via the legacy `Eip712Signer.signDelegation` static, which rebuilt the typed-data `types` VERBATIM from the backend payload. A compromised/MITM backend appending a hidden field to `Delegation`/`Caveat` (e.g. `secretApproval uint256`) had it silently signed by the device. Route signing through the hardened `signDelegationEnvelope`, which pins `types` against the client schema and re-validates the trusted params before any byte reaches the BitBox. On the happy path the signed envelope is byte-identical, so the signature still verifies backend-side. Test: an injected Delegation field is now refused with Eip712SchemaDriftException; canonical types still sign. F2 (MED) — `salt` (uint256) and `userNonce` (uint64) were parsed as Dart `int`, silently truncating/overflowing values beyond 2^63 (2^53 on web) — the signed salt no longer matched what the backend issued. Parse both as BigInt (number-or-string tolerant) and serialise salt as a decimal string at the EIP-712/DTO boundary (uint256-equivalent, byte-safe). Tests cover a full-width uint256 salt and a uint64 nonce beyond int range. F3 (LOW) — the romanisation invariant test only checked for pure ASCII, which the `?` last-resort placeholder satisfies — so silent transliteration loss slipped through. Assert no `?` placeholder for inputs without one, and pin the placeholder contract for a genuinely unmappable rune. Fixing the revealed gap, also map the Swiss-French/Italian guillemets « » ‹ › (they were degrading to `?`). F4 (LOW) — the EIP-712 domain `name`/`version` (which feed the domain separator the user signs) were unvalidated. Pin them in `signDelegationEnvelope` when the caller supplies the expected values, and wire the RealUnit DelegationManager domain (RealUnit / 1) at the sell caller. Tests cover name + version drift and the matching happy path. All red→green verified (each fix proven to fail when reverted/mutated); flutter analyze clean; 258 sign/sell/integration tests green.
… --fatal) Dropping const from the outer Eip7702Data/SellPaymentInfo fixtures (needed for the new BigInt salt/userNonce fields) left the const-able inner constructors flagged by prefer_const_constructors. CI's flutter analyze fails on info-level lints; restore const on the domain/types/beneficiary constructors that do not contain a BigInt.
…ture Two issues the full suite surfaced for the RealUnitCH#608 sign-pipeline fixes: - The EIP-7702 authorization-confirm DTO echoes the nonce as a JSON number (the backend contract pinned by the existing confirm test). Send userNonce.toInt() instead of a string; the security-critical path still signs the exact BigInt nonce via signAuthorization. - real_unit_sell_payment_info_service_confirm_test's software-wallet happy path actually signs, so now that confirmPayment routes through the schema-pinned signDelegationEnvelope the fixture must carry the canonical Delegation/Caveat types AND real hex (a 20-byte relayer, a 32-byte authority) the EIP-712 encoder can ABI-encode — placeholders like '0xrelay'/'0xauth' only worked on the legacy verbatim path. Full suite: flutter analyze clean, 2407 tests (excl golden) green.
TaprootFreak
force-pushed
the
split/pr578-03-sign-pipeline
branch
from
8554cc7 to
e5f420e
Compare
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.
Draft split from #578 after TaprootFreak review.
Scope:
Validation: