MorphTx enhance & implement reference key#282
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a versioned MorphTx (v0/v1) with Reference and Memo fields, replaces AltFeeTx paths, persists and indexes MorphTx references (background indexer), and propagates Version/Reference/Memo through transactions, receipts, RPC, pool, state, and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant RPC as RPC
participant TxPool as TxPool
participant Blockchain as Blockchain
participant State as State
participant RefIdx as RefIdx
participant DB as DB
User->>RPC: SendTransaction(MorphTx with Reference)
RPC->>TxPool: SubmitTx
TxPool->>TxPool: ValidateMorphTxVersion(), ValidateMemo()
TxPool-->>RPC: Accepted
Blockchain->>State: ApplyTransaction(MorphTx)
State->>State: Create Receipt (Version, Reference, Memo)
State-->>Blockchain: Receipt
Blockchain->>RefIdx: WriteReferenceIndexEntriesForBlock(block)
RefIdx->>DB: WriteReferenceIndexEntry(...)
RefIdx->>DB: WriteReferenceIndexTail(...)
User->>RPC: GetTransactionHashesByReference(reference, offset, limit)
RPC->>DB: ReadReferenceIndexEntries(reference)
DB-->>RPC: ReferenceIndexEntry[]
RPC-->>User: ReferenceTransactionResult[]
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
core/rawdb/accessors_chain.go (1)
631-645:⚠️ Potential issue | 🔴 CriticalFix type mismatches to align with core/types/receipt.go canonical definition.
The struct has type inconsistencies with the canonical
storedReceiptRLPdefinition incore/types/receipt.go:
Memoshould be*[]byte(pointer), not[]byte(direct slice) — the canonical definition and all RLP encoding/decoding logic use the pointer typeVersionshould beuint8for consistency with the canonical definition (byte is an alias, but uint8 is the standard notation used throughout core/types)These mismatches will cause encoding/decoding issues since the canonical definition in
core/types/receipt.gouses*[]byteforMemoin bothstoredReceiptRLPandv8StoredReceiptRLP.core/tx_list.go (1)
401-419:⚠️ Potential issue | 🟠 MajoraltCostLimit isn’t enforced for MorphTxWithAltFee.
allLoweris computed but never used in theIsMorphTxWithAltFeebranch, so alt-token balance limits won’t trigger eviction. If that’s unintentional, incorporate the limit into the predicate (or compare per‑tx alt cost).🧭 Minimal fix to apply alt-cost caps
if tx.IsMorphTxWithAltFee() { for id, limit := range altCostLimit { lower := l.costcap.Alt(id).Cmp(limit) <= 0 if !lower { l.costcap.SetAltAmount(id, limit) } allLower = allLower && lower } - return tx.Gas() > gasLimit || tx.Value().Cmp(costLimit) > 0 + return !allLower || tx.Gas() > gasLimit || tx.Value().Cmp(costLimit) > 0 }core/blockchain.go (1)
1978-1994:⚠️ Potential issue | 🟠 MajorReorgs don’t remove reference‑index entries.
Tx lookup entries are deleted, but reference entries for removed blocks remain, leaving stale reference lookups after reorgs. Consider deleting reference entries for oldChain blocks in the same batch.🧩 Suggested fix
indexesBatch := bc.db.NewBatch() +for _, block := range oldChain { + rawdb.DeleteReferenceIndexEntriesForBlock(indexesBatch, block) +} for _, tx := range types.TxDifference(deletedTxs, addedTxs) { rawdb.DeleteTxLookupEntry(indexesBatch, tx.Hash()) }core/types/transaction_marshalling.go (1)
576-611:⚠️ Potential issue | 🔴 CriticalGuard
dec.Versionbefore dereferencing to prevent nil pointer panic.The decoder receives
Versionas a*uint8pointer. If JSON omits the"version"field,dec.Versionis nil, causing a panic at line 608 when dereferenced without a guard. All other required fields (ChainID, Nonce, Gas, etc.) are validated before use; Version must follow the same pattern.Default to
MorphTxVersion0if the field is absent:Suggested fix
- itx.Version = *dec.Version + if dec.Version == nil { + itx.Version = MorphTxVersion0 + } else { + itx.Version = *dec.Version + }core/types/transaction.go (1)
450-459:⚠️ Potential issue | 🟠 Major
Cost()panics for MorphTx transactions.Calling
Cost()on a MorphTx will cause a panic. While there's a TODO comment indicating this is intentional for now, this is a significant correctness risk. Any code path that callsCost()on a MorphTx without first checkingIsMorphTx()will crash.Consider one of:
- Return an error instead of panicking
- Document this clearly in the method's godoc
- Implement proper cost calculation for MorphTx without alt-fee
🐛 Consider returning error or calculating cost
Option 1 - Return error (requires signature change):
func (tx *Transaction) Cost() (*big.Int, error) { if tx.IsMorphTx() { return nil, ErrCostNotSupported } total := tx.GasFee() total.Add(total, tx.Value()) return total, nil }Option 2 - Calculate cost for MorphTx without alt-fee:
func (tx *Transaction) Cost() *big.Int { - // TODO: morph tx without fee token - if tx.IsMorphTx() { + if tx.IsMorphTxWithAltFee() { panic(ErrCostNotSupported) } total := tx.GasFee() total.Add(total, tx.Value()) return total }
🤖 Fix all issues with AI agents
In `@accounts/abi/bind/base.go`:
- Around line 340-358: The current version-selection logic (where version :=
opts.Version; if version == 0 && opts.FeeTokenID == 0 { version =
types.MorphTxVersion1 }) can leave Version 0 when FeeTokenID > 0 and
accidentally drop Reference/Memo; change it so that when opts.Version == 0 you
also force types.MorphTxVersion1 if either opts.Reference or opts.Memo is
non-nil (treat non-nil pointers as explicit intent even if empty), then build
the types.MorphTx (baseTx) with that version; apply the same fix to the second
occurrence of the selection logic (the other block around the later lines) so
both code paths respect Reference/Memo presence.
In `@accounts/external/backend.go`:
- Around line 224-234: The code dereferences tx.Memo() when handling
types.MorphTxType which can be nil; update the MorphTxType branch in the switch
to check if tx.Memo() != nil before converting/dereferencing it and assigning to
args.Memo (e.g., only create hexutil.Bytes(*tx.Memo()) and set args.Memo when
tx.Memo() is non-nil). Keep other assignments unchanged; mirror the guard
pattern used in ValidateMemo()/AsMorphTx() and other callers to avoid a panic.
In `@common/types.go`:
- Around line 124-131: Reference.SetBytes currently left-aligns the input which
breaks numeric/hex round-trips and differs from Hash.SetBytes; change it to
right-align and zero any leading bytes: if len(b) > len(r) crop b from the left
as before, otherwise clear r (set all bytes to 0) and copy b into the trailing
bytes of r so the least-significant bytes are right-aligned; update the
implementation in the Reference.SetBytes method to perform the zeroing and
tail-copy (use the existing Reference type and ReferenceLength/len(r) to locate
the copy destination).
In `@core/rawdb/reference_index_iterator.go`:
- Around line 104-113: The indexing stalls because when reading from rlpCh in
the loop (the block with header/rlp decode), the code uses continue on errors
and never sends a result for that block to resultCh, leaving gaps that make
indexReferences' priority check (the priority vs lastNum-1 logic) wait forever;
fix by emitting a placeholder/empty result into resultCh whenever data.header ==
nil or rlp.DecodeBytes fails (preserving data.number), so the priority queue
still receives a contiguous entry for that block (include any error metadata in
the placeholder), i.e., in the loop that decodes into types.Body and handles
errors, create and send an empty/failed BlockReference result for the given
block number to resultCh instead of continuing.
In `@core/types/gen_receipt_json.go`:
- Around line 172-174: The generated truncation occurs because receiptMarshaling
uses hexutil.Uint64 for the Type and Version fields which are later cast to
uint8 in gen_receipt_json.go; update the source marshaling struct in
receiptMarshaling (in core/types/receipt.go) to use a narrower type (e.g., add
and use hexutil.Uint8) or add validation at the top-level unmarshaling
entrypoint (e.g., the receipt UnmarshalJSON/Unmarshal function) to reject values
>255 before the generated code runs, and apply the same change for both Type and
Version to prevent silent truncation.
In `@core/types/l2trace.go`:
- Around line 204-217: Guard against nil dereference of tx.Memo() in the MorphTx
handling block: before converting memo to hexutil.Bytes and assigning to
result.Memo, check if tx.Memo() != nil; only then do the conversion (memo :=
hexutil.Bytes(*tx.Memo())) and set result.Memo = &memo, otherwise leave
result.Memo nil. Update the block that checks tx.Type() == MorphTxType (and uses
tx.Version(), tx.Reference(), FeeTokenID, FeeLimit) to include this nil-check
for tx.Memo().
In `@core/types/morph_tx.go`:
- Around line 181-227: The V0 encode path in MorphTx.encode lacks the same
FeeTokenID != 0 validation present in decodeV0MorphTxRLP, causing asymmetry; add
a check at the start of the MorphTxVersion0 branch in encode (inside
MorphTx.encode) to return an error if tx.FeeTokenID == 0 (with a clear message
like "invalid FeeTokenID for v0 morph tx") before calling rlp.Encode, ensuring
behavior matches decodeV0MorphTxRLP and prevents encoding invalid v0 payloads.
In `@core/types/transaction_test.go`:
- Around line 732-738: The test currently compares Sender errors with direct
equality (in the table-driven loop using tc.wantSenderErr) which fails for
wrapped errors; update the assertion to use errors.Is(err, tc.wantSenderErr)
instead of err == tc.wantSenderErr and adjust the test import to include the
"errors" package if not present; locate the comparison inside the t.Run block
where Sender(signer, tc.tx) is called and replace the equality check and error
message accordingly.
In `@core/types/transaction.go`:
- Around line 974-981: The comment for copyReferencePtr is wrong—replace
"copyAddressPtr copies an address" with a proper Go doc comment that starts with
the function name and describes what it does, e.g. "copyReferencePtr copies a
common.Reference and returns a pointer to the copy"; update the comment to
reference common.Reference and ensure it follows Go comment conventions for
exported identifiers.
In `@ethclient/ethclient.go`:
- Around line 375-384: GetTransactionHashesByReference is a public method but
returns internal type ethapi.ReferenceTransactionResult (from internal/ethapi),
which external callers cannot use; define a public result type in the ethclient
package (e.g. type ReferenceTransactionResult struct { ... } matching the fields
of ethapi.ReferenceTransactionResult), then change
GetTransactionHashesByReference to return
[]ethclient.ReferenceTransactionResult; inside the method, call the RPC into a
local slice of ethapi.ReferenceTransactionResult as currently done, map/convert
each ethapi.ReferenceTransactionResult to the new
ethclient.ReferenceTransactionResult and return that slice (keep function name
GetTransactionHashesByReference and RPC call
"morph_getTransactionHashesByReference" unchanged).
In `@internal/ethapi/transaction_args.go`:
- Around line 373-387: The code unconditionally sets MorphTx metadata (version,
reference, memo) before calling types.NewMessage, causing non‑Morph calls to be
treated as MorphTx; change the logic in the ToMessage (or the containing
function) to only initialize and pass version, reference, and memo when
args.isMorphTxArgs() returns true — otherwise leave them nil/defaults — and then
call types.NewMessage with those conditional values so non‑Morph calls keep
their original encoding/behavior.
In `@signer/core/apitypes/types.go`:
- Around line 124-153: The code casts *args.Version to uint8 for
types.MorphTx.Version which can silently truncate large hexutil.Uint64 values;
before assigning do an explicit range check on args.Version (e.g., ensure
*args.Version <= 255) and return/propagate an error if out of range, or change
the source type to a uint8-typed value; update the block that sets version
(checking args.Version != nil) to validate the numeric range and only cast when
safe so types.MorphTx.Version cannot receive a truncated value.
🧹 Nitpick comments (9)
core/blockchain_test.go (1)
3590-4063: Avoid fixed sleeps for index initialization to reduce test flakiness.
time.Sleep(50ms)can be brittle on slow CI. Consider polling for expected entry counts or reference-index tail with a timeout helper instead.core/types/transaction_marshalling.go (1)
65-70: Consider omitting MorphTx fields when unused.
Withoutomitempty, non‑MorphTx JSON includes"version": null,"reference": null,"memo": null. If that’s not intended, addomitemptyto keep output stable.🔧 Suggested tweak
- Version *uint8 `json:"version"` - Reference *common.Reference `json:"reference"` - Memo *hexutil.Bytes `json:"memo"` + Version *uint8 `json:"version,omitempty"` + Reference *common.Reference `json:"reference,omitempty"` + Memo *hexutil.Bytes `json:"memo,omitempty"`core/types/receipt.go (1)
121-151:storedReceiptRLPandv8StoredReceiptRLPare identical structures.Both structs have the exact same fields. If they're meant to be identical for v8, consider removing the duplication. If v8 is supposed to differ from the current format, one of them likely has the wrong fields.
Additionally, in
DecodeRLP(lines 397-402),decodeStoredReceiptRLPis tried first, and if it fails,decodeV8StoredReceiptRLPis tried. Since both decode to identical structures, the v8 decoder will likely never succeed if the first one failed (and vice versa).♻️ Consider consolidating or differentiating the structs
If v8 is the current format:
-// storedReceiptRLP is the storage encoding of a receipt. -type storedReceiptRLP struct { - PostStateOrStatus []byte - CumulativeGasUsed uint64 - Logs []*LogForStorage - L1Fee *big.Int - FeeTokenID *uint16 - FeeRate *big.Int - TokenScale *big.Int - FeeLimit *big.Int - Version uint8 - Reference *common.Reference - Memo *[]byte -} - -// v8StoredReceiptRLP is the storage encoding of a receipt used in database version 8. -// This version was introduced when MorphTx feature was added. -// It includes L1Fee and all MorphTx fields (FeeTokenID, FeeRate, TokenScale, FeeLimit, Version, Reference, Memo). -type v8StoredReceiptRLP struct { +// storedReceiptRLP is the storage encoding of a receipt used in database version 8. +// This version was introduced when MorphTx feature was added. +// It includes L1Fee and all MorphTx fields (FeeTokenID, FeeRate, TokenScale, FeeLimit, Version, Reference, Memo). +type storedReceiptRLP struct { PostStateOrStatus []byte CumulativeGasUsed uint64 Logs []*LogForStorage L1Fee *big.Int FeeTokenID *uint16 FeeRate *big.Int TokenScale *big.Int FeeLimit *big.Int Version uint8 Reference *common.Reference Memo *[]byte }Then remove
decodeV8StoredReceiptRLPand the corresponding call inDecodeRLP.core/types/morph_tx.go (1)
1-28: Copyright header is placed after the package declaration and imports.The copyright notice should appear at the very top of the file, before the
packagestatement. This is both a Go convention and ensures proper license attribution.♻️ Move copyright to top of file
+// Copyright 2021 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>. + package types import ( "bytes" "errors" "math/big" "strconv" "github.com/morph-l2/go-ethereum/common" "github.com/morph-l2/go-ethereum/rlp" ) - -// Copyright 2021 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.internal/ethapi/api.go (2)
1846-1876: Inconsistent error handling for pagination edge cases.When
len(entries) == 0, the method returnsnil, nil(line 1847-1849). However, whenoffsetVal >= len(entries), it returns an error (line 1852-1854). This inconsistency may confuse API consumers:
- Empty reference →
nilresult- Valid reference but offset too high → error
Consider returning an empty slice for both cases, or documenting this behavior clearly.
Additionally, when
ReadTxLookupEntryreturnsnil(line 1866-1869), entries are silently skipped. This could result in fewer results than expected without any indication to the caller.♻️ Consider more consistent error handling
entries := rawdb.ReadReferenceIndexEntries(s.b.ChainDb(), reference) if len(entries) == 0 { - return nil, nil + return []ReferenceTransactionResult{}, nil } // Validate offset if offsetVal >= uint64(len(entries)) { - return nil, fmt.Errorf("offset %d exceeds total results %d", offsetVal, len(entries)) + return []ReferenceTransactionResult{}, nil }
1974-1979: MorphTx receipt fields are always included regardless of transaction type.The
version,reference, andmemofields are included in the receipt response for all transaction types. For non-MorphTx transactions, these will be0/nil. Consider conditionally including these fields only forMorphTxTypeto reduce response payload size and avoid confusion.core/rawdb/reference_index_iterator.go (3)
50-53: Edge case:from == toreturns nil channel, preventing single-block indexing.If someone wants to index exactly one block (e.g.,
IndexReferences(db, 100, 101, nil)), this works. Butfrom == toreturns nil, which could cause nil pointer issues in callers if not handled. The comment says "short circuit for invalid range" butfrom == tomeaning "index zero blocks" is valid behavior.Consider returning a closed channel instead of nil for safer handling by callers.
♻️ Return closed channel for empty range
func iterateReferences(db ethdb.Database, from uint64, to uint64, reverse bool, interrupt chan struct{}) chan *blockReferenceInfo { if to == from { - return nil + ch := make(chan *blockReferenceInfo) + close(ch) + return ch }
128-141: Inconsistent indentation in the process function.Lines 128-141 have inconsistent indentation compared to lines 115-127 within the same function scope. This affects readability.
📝 Fix indentation
} } - // Always send result for every block (even if no references) to maintain - // contiguous block numbers for gap-filling logic in indexReferences - result := &blockReferenceInfo{ - number: data.number, - blockTimestamp: data.header.Time, - references: refs, - } - // Feed the block to the aggregator, or abort on interrupt - select { - case resultCh <- result: - case <-interrupt: - return - } + // Always send result for every block (even if no references) to maintain + // contiguous block numbers for gap-filling logic in indexReferences + result := &blockReferenceInfo{ + number: data.number, + blockTimestamp: data.header.Time, + references: refs, + } + // Feed the block to the aggregator, or abort on interrupt + select { + case resultCh <- result: + case <-interrupt: + return + } } }
285-295: Hardcoded flush frequency differs from indexing path.The indexing path uses
ethdb.IdealBatchSize(line 200) for batch flushing, but unindexing uses a hardcodedblocks%1000 == 0(line 288). The comment explains deletions count as size 1, but consider making this configurable or using a named constant for consistency and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/ethapi/transaction_args.go`:
- Around line 87-95: The isMorphTxArgs method on TransactionArgs currently omits
FeeLimit, so add a check for the FeeLimit field (e.g., args.FeeLimit != nil) to
the boolean expression in TransactionArgs.isMorphTxArgs so that transactions
with only FeeLimit set are detected as MorphTxs; update the return expression to
include the FeeLimit check alongside FeeTokenID, Version, Reference, and Memo.
- Around line 107-111: The code reads args.Version (a *hexutil.Uint16) and
directly casts it to uint8 into variable version (defaulting to
types.MorphTxVersion1), which can silently truncate values >255; add explicit
validation where args.Version is handled: check that args.Version != nil and
that uint16(*args.Version) is <= 1 (allowed values 0 or 1), otherwise return a
descriptive error (e.g., "invalid Version: must be 0 or 1"); only after this
validation assign to version (cast to uint8). Ensure you reference args.Version,
the local variable version, and types.MorphTxVersion1 when applying the change.
🧹 Nitpick comments (1)
internal/ethapi/api.go (1)
1822-1877: Avoid loading full reference index before pagination.
ReadReferenceIndexEntriesloads all entries then slices, which can be heavy for hot references. Consider an iterator that skips/limits in‑stream; apply the same approach to the PublicMorphAPI method.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/ethapi/transaction_args.go`:
- Around line 137-139: The current check only rejects positive FeeLimit when
feeTokenID == 0; change the condition to reject any non-zero FeeLimit (i.e.,
check args.FeeLimit.ToInt().Sign() != 0) so negative values are also rejected
and return types.ErrMorphTxV1FeeLimitWithoutFeeToken; apply the same fix to the
analogous check around the other occurrence (the second check referenced in the
comment).
🧹 Nitpick comments (1)
internal/ethapi/transaction_args.go (1)
464-468: Extract duplicated MorphTx detection logic.Lines 464-467 duplicate the condition from
isMorphTxArgs(). If detection criteria change, both locations must be updated.♻️ Proposed refactor
switch { // must take precedence over MaxFeePerGas. - case (args.FeeTokenID != nil && *args.FeeTokenID > 0) || - (args.Version != nil) || // Any explicit version setting indicates MorphTx intent - (args.Reference != nil && *args.Reference != (common.Reference{})) || - (args.Memo != nil && len(*args.Memo) > 0): + case args.isMorphTxArgs(): usedType = types.MorphTxType
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/ethapi/transaction_args.go (1)
313-404:⚠️ Potential issue | 🟠 MajorValidate MorphTx version and memo length in ToMessage.
ToMessagecan be called withoutsetDefaults, so invalidVersion/Memovalues can bypass validation and be truncated or overlong. Consider running the same validation used bysetDefaults.🛡️ Suggested validation
func (args *TransactionArgs) ToMessage(globalGasCap uint64, baseFee *big.Int) (types.Message, error) { + if args.Memo != nil && len(*args.Memo) > common.MaxMemoLength { + return types.Message{}, errors.New("memo exceeds maximum length of 64 bytes") + } + if err := args.validateMorphTxVersion(); err != nil { + return types.Message{}, err + } // Reject invalid combinations of pre- and post-1559 fee stylescore/tx_pool.go (1)
1628-1669:⚠️ Potential issue | 🟠 MajorEnsure gas-limit and alt-fee balance checks apply to MorphTxWithAltFee.
txCostis nil for alt-fee MorphTx, so the initial gas-limit check is skipped. Also, when FeeVault is disabled, alt-fee balance checks don’t run at all. This can leave unexecutable txs in the pool when gas limits or balances change.🛡️ Suggested fix
- if txCost != nil && (tx.Gas() > pool.currentMaxGas || txCost.Cmp(costLimit) > 0) { + if tx.Gas() > pool.currentMaxGas { + return true + } + if txCost != nil && txCost.Cmp(costLimit) > 0 { return true } + // If FeeVault is disabled, still enforce alt-fee balance/value checks + if !pool.chainconfig.Morph.FeeVaultEnabled() && tx.IsMorphTxWithAltFee() { + altAmount, err := fees.EthToAlt(pool.currentState, tx.FeeTokenID(), tx.GasFee()) + if err != nil { + log.Error("Failed to swap to erc20", "err", err, "tx", tx) + return false + } + limit := altCostLimit[tx.FeeTokenID()] + if tx.FeeLimit() != nil && tx.FeeLimit().Sign() > 0 { + limit = cmath.BigMin(limit, tx.FeeLimit()) + } + return costLimit.Cmp(tx.Value()) < 0 || limit.Cmp(altAmount) < 0 + }
🤖 Fix all issues with AI agents
In `@accounts/abi/bind/base.go`:
- Around line 372-394: When opts.Version is nil you currently return
types.MorphTxVersion1 without validating FeeLimit vs FeeTokenID; add the same
check used in the types.MorphTxVersion1 case so auto-selected v1 cannot have
FeeTokenID==0 while FeeLimit is non-zero. Specifically, in the branch where
opts.Version == nil before returning types.MorphTxVersion1, inspect
opts.FeeTokenID and opts.FeeLimit (and FeeLimit.Sign()) and return
types.ErrMorphTxV1IllegalExtraParams if FeeTokenID == 0 && opts.FeeLimit != nil
&& opts.FeeLimit.Sign() != 0.
In `@core/tx_list.go`:
- Around line 407-415: The check for MorphTx leaves txCost nil and causes a
panic when calling txCost.Cmp; update the MorphTx branch in the filtering
closure so that when tx.IsMorphTx() is true but the specialized path (e.g.,
IsMorphTxWithAltFee) is not applicable you assign txCost = tx.Cost() as a safe
fallback (or otherwise compute a non-nil cost) before the final return that
compares txCost.Cmp(costLimit); ensure you still compute txCost differently only
when the specialized MorphTx fee logic is available so txCost is never nil when
used in the return expression along with tx.Gas(), gasLimit and costLimit.
In `@core/tx_pool_test.go`:
- Around line 3008-3012: The test incorrectly constructs tx2 using morphTxV1
with identical inputs so it matches tx1 and triggers "already known" instead of
testing same-price replacement; change the tx2 construction to differ in a
non-fee field (for example change Gas, Value, or Data) while keeping the same
gas price so addRemoteSync(tx2) exercises the "same-price replacement" rejection
path — locate the test where tx1 is created and the subsequent tx2 line (using
morphTxV1) and modify tx2 to vary one non-fee parameter while preserving the fee
fields.
🧹 Nitpick comments (1)
signer/core/apitypes/types.go (1)
124-127: Include FeeLimit in MorphTx detection to avoid silent drop.
FeeLimitis MorphTx-specific; if it’s the only Morph field set, this branch won’t be chosen and the field gets ignored. Consider treatingFeeLimitas MorphTx intent too.♻️ Suggested adjustment
-case (args.FeeTokenID != nil && *args.FeeTokenID > 0) || +case (args.FeeTokenID != nil && *args.FeeTokenID > 0) || + (args.FeeLimit != nil) || (args.Version != nil) || (args.Reference != nil && *args.Reference != (common.Reference{})) || (args.Memo != nil && len(*args.Memo) > 0):
txJSON.Version was *uint8 (plain number) while RPCTransaction.Version uses *hexutil.Uint64 (hex string). This caused ethclient.BlockByNumber to fail with "cannot unmarshal string into Go struct field rpcBlock.transactions.version of type uint8" on non-sequencer nodes. Align txJSON.Version with the Ethereum JSON-RPC convention by using *hexutil.Uint64, consistent with all other numeric fields (FeeTokenID, QueueIndex, etc.).
5 participants
Summary by CodeRabbit
New Features
Enhancements
Tests