IntersectMBO / IntersectMBO/evolution-sdk

Signing: signTx hashes a re-encoded body when given a Transaction object

Aperta
#531 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
bug external-review
Lingua principale
TypeScript
Stelle
22
Fork
30
Merge medio
5h 29m
PR unite (30g)
12

Descrizione

## Summary
`signTx` accepts `Transaction | string` and computes the message to sign two different ways. Given a
hex string it hashes the original body bytes; given a decoded `Transaction` it calls
`TransactionBody.toHash(tx.body)`, which re-encodes the body canonically. Since #236, submission
replays the original bytes through the `formatCache`, so for a body decoded from a valid but
non-canonical encoding (indefinite-length arrays, non-minimal integers) the object path signs a
transaction id the node will not compute, and the witness does not validate. The cause is the lookup:
`formatCache` is keyed on `Transaction` (Transaction.ts L130), so passing `tx.body` cannot find the
cached format, while `tx` itself is in scope on the line above.

## Affected
packages/evolution/src/sdk/client/internal/Signing.ts
- signTx message selection (L328-331): string branch hashes original bytes via `toHashFromBytes`;
object branch re-encodes via `TransactionBody.toHash(tx.body)` (L331), while `tx` is in scope (L323)

packages/evolution/src/Transaction.ts
- formatCache (L130) keyed on `Transaction`; written only by `fromCBORBytes` (L141) and
`fromCBORHex` (L150), transferred through `addVKeyWitnesses` (L363-364)
- contrast: the object call sites in sdk/builders/SignBuilderImpl.ts (L119, L281) are fed only by
sdk/builders/internal/build.ts (L41) with freshly built transactions, which carry no cached format,
so the two paths agree there

## Fix
Make the object overload hash the bytes that will actually be submitted, so both overloads agree:

: TransactionBody.toHashFromBytes(Transaction.extractBodyBytes(Transaction.toCBORBytes(tx)))

One caveat before applying it. Hashing original bytes means signing entries the decoded body dropped,
including duplicate CBOR map keys, which `decodeMapAt` currently accepts with last-writer-wins. That
exposure exists on the string path today and this change extends it to both. Pair it with rejecting
duplicate keys on decode, the same fail-closed approach taken for Data in #397, or the fix trades one
inconsistency for a wider one.

## Regression test
- given: a transaction decoded from a body with a non-minimal fee (`1a0000000a` where canonical is `0a`)
that round-trips exactly through `Transaction.toCBORBytes`
- before fix: the object overload signs `233fb742...` while the node txid over the submitted bytes is
`1d043700...`; the string overload signs `1d043700...`
- after fix: object overload, string overload, and node txid all agree
- control: a fully canonical body must match on all three before and after

Must FAIL on main today and PASS after the fix.

## Reference
Residual inconsistency after #236 made submission byte-preserving while the object signing path kept
canonicalizing. Adjacent to #235, the same class in redeemer PlutusData, and to #480, the same
re-encode-instead-of-preserve pattern in COSE.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Start in packages/evolution/src/sdk/client/internal/Signing.ts at signTx lines 323-331, then inspect the formatCache handling in packages/evolution/src/Transaction.ts lines 130-150 and 363-364. Add the described regression test using a non-minimal fee encoding, plus the canonical control, and verify that object signing, string signing, and the node transaction id agree after the fix.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
typescript
Ambito
blockchain, cryptography
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Attiva
Chiarezza
Specificata chiaramente
Idoneità per principianti
76/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.