Skip to content

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

Description

@solidsnakedev

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingexternal-reviewFrom external review batch

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions