feat(abstract-eth): expose getSignablePayload for ETH coin classes (CGD-1083)#8826
Conversation
dbc8c82 to
14a7783
Compare
There was a problem hiding this comment.
🔴 Token coins throw on getSignablePayload — contradicts PR description
modules/abstract-eth/src/ethLikeToken.ts:406-408: EthLikeToken.getTransactionBuilder() is stubbed to throw new Error('Method not implemented'). Because EthLikeToken extends AbstractEthLikeNewCoins extends AbstractEthLikeCoin, it inherits the new override at abstractEthLikeCoin.ts:234-239, whose first line is this.getTransactionBuilder(). Calling <token>.getSignablePayload(serializedTx) now throws 'Method not implemented'.
Why this blocks: The PR description explicitly lists tokens as inheritors of this behavior ("Inherited by all AbstractEthLikeNewCoins descendants (Eth, Polygon, Arbeth, Opeth, BSC, AvaxC, EVM, tokens, etc.)"). Today the base-class default at least returned a Buffer (broken UTF-8, but non-throwing); now any ERC-20 path through AKM's /sign will hard-fail with an opaque error.
Suggested fix: Either
- (a) override
getSignablePayloadonEthLikeTokento construct a vanilla ETH-coin builder — the wire-level tx is just an ETH tx, the token semantics live in the calldata; or - (b) explicitly throw a clear
NotImplementedErrorso callers can fall back, and update the PR description to exclude tokens.
In either case, please add a token unit test mirroring the new eth.ts end-to-end tests (e.g. tdai) so this regression is caught by CI.
326f673 to
0547142
Compare
Override `getSignablePayload` on `AbstractEthLikeCoin` to return the keccak256 hash of the unsigned transaction (via `getMessageToSign(true)`) rather than raw serialized bytes. This exposes the correct bytes AKM needs for its external POST /sign endpoint. Changes: - Add `getSignablePayload(): Buffer` to `EthLikeTransactionData` interface and implement it in `EthTransactionData` using `tx.getMessageToSign(true)` - Add `get signablePayload(): Buffer` to the `Transaction` class, delegating to `EthTransactionData.getSignablePayload()` - Override `getSignablePayload(serializedTx)` in `AbstractEthLikeCoin`, rebuilding via the transaction builder and returning `tx.signablePayload` - Add unit tests covering Legacy and EIP1559 transaction types, and empty-transaction error handling Ticket: CGD-1083
0547142 to
765df32
Compare
Description
Overrides
getSignablePayload(serializedTx)onAbstractEthLikeCoinso that ETH and all ETH-like coins return the correct 32-byte keccak256 signing digest, rather than falling through to the base-class no-op that returns raw serialized bytes.This is required to unblock AKM's external signing endpoint (
POST /sign), which needs the correct hash to sign — not the raw transaction bytes.Changes
AbstractEthLikeCoin.getSignablePayload— new override: deserializes via the transaction builder, casts toEthTransaction, and returnstx.signablePayloadEthLikeTransactionDatainterface — addsgetSignablePayload(): BufferEthTransactionData— implementsgetSignablePayload()viatx.getMessageToSign(true)(keccak256 hash, same primitive used internally for TSS signing)Transaction.signablePayloadgetter — overridesBaseTransaction'sNotImplementedErrordefault; throwsInvalidTransactionErrorif no transaction data is setEthLikeToken.getSignablePayload— preserves pre-PR behavior (returns raw serialized bytes); tokens are out of scope for this ticketAbstractEthLikeNewCoinsdescendants (Eth, Polygon, Arbeth, Opeth, BSC, AvaxC, EVM, etc.) and legacyAbstractEthLikeCoindescendants (Rbtc, Celo, Etc)Issue
CGD-1083 (unblocks WCN-397)
Type of change
How Has This Been Tested?
Unit tests added in:
modules/abstract-eth/test/unit/transaction.ts— throws on empty tx; returns 32-byte hash for unsigned txmodules/sdk-coin-eth/test/unit/transaction.ts— hash pinned againsttxData.idfixture for both Legacy and EIP1559; distinct-payload check between tx typesmodules/sdk-coin-eth/test/unit/eth.ts— end-to-endcoin.getSignablePayload(serializedTx)for Legacy and EIP1559 viaTethExisting TSS signing flows are unaffected — they use
signableHexfrom the unsigned tx object directly (abstractEthLikeNewCoins.ts:2444), notgetSignablePayload.