Conversation
|
👋 wentzeld, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
There was a problem hiding this comment.
Pull request overview
This PR is focused on improving the clarity and actionability of error messages across the EVM RPC client, tx manager components, simulated backend, and contract read layer.
Changes:
- Makes RPC client and txmgr errors more descriptive (configuration hints, operational guidance, unsupported-chain messaging).
- Refines contract read / logpoller error strings to better identify failure source and context.
- Updates RPC client tests to assert on the new error messages.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/txmgr/transmitchecker.go | Improves VRF transmit-checker configuration and “already fulfilled” error messages. |
| pkg/txmgr/stuck_tx_detector.go | Expands missing-config error for stuck-tx heuristic. |
| pkg/txmgr/finalizer.go | Replaces the “could not get receipt” saved error string with a more detailed operator-facing message. |
| pkg/txmgr/evm_tx_store.go | Adds more context to tx attempt/tx state transition errors. |
| pkg/txmgr/attempts.go | Improves errors for force-rebroadcast attempt creation and EIP-1559 fee validation failures. |
| pkg/read/errors.go | Clarifies contract read, batch read, configuration, filter, and “no contract at address” error messages. |
| pkg/client/simulated_backend_client.go | Makes simulated-backend argument validation errors more descriptive. |
| pkg/client/rpc_client_test.go | Updates expectations for new RPC client error strings. |
| pkg/client/rpc_client_internal_test.go | Updates expected doWithConfidence error string. |
| pkg/client/rpc_client.go | Improves dial/subscription/unsupported-chain error messages and Astar batch-call handling errors. |
| pkg/client/limited_transport.go | Expands “response too large” error text with remediation guidance. |
| pkg/client/errors.go | Expands terminally-stuck message and RPC error extraction error messages. |
| pkg/client/compatibility_helper.go | Improves invalid filter-query error message when BlockHash and range are both set. |
Comments suppressed due to low confidence (1)
pkg/client/errors.go:314
TerminallyStuckMsgis used both as the stored Tx.Error string and as the regex pattern (regexp.MustCompile(TerminallyStuckMsg)). Changing the constant will stop matching previously persisted rows that used the old value, potentially changing how older transactions are classified on upgrade. Consider decoupling the stored message from the matching pattern (e.g., use a regex that matches both old and new forms) to preserve backward compatibility.
const TerminallyStuckMsg = "transaction is terminally stuck in the mempool and cannot be included in a block: this transaction will not be retried and must be investigated manually"
// Tx.Error messages that are set internally so they are not chain or client specific
var internal = ClientErrors{
TerminallyStuck: regexp.MustCompile(TerminallyStuckMsg),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case *big.Int: | ||
| if v.Uint64() > 0 || len(v.Bytes()) > 20 { | ||
| return common.Address{}, errors.New("invalid *big.Int; value must be larger than 0 with a byte length <= 20") | ||
| return common.Address{}, errors.New("invalid *big.Int for address conversion: value must be greater than 0 with a byte length of 20 or fewer bytes to represent a valid EVM address") | ||
| } |
There was a problem hiding this comment.
In the *big.Int address conversion branch, the validation condition v.Uint64() > 0 || len(v.Bytes()) > 20 rejects every positive value, making virtually all non-zero addresses invalid. This looks inverted; it should reject negative values (and/or values whose byte length exceeds 20), not values greater than 0.
pkg/txmgr/attempts.go
Outdated
|
|
||
| if fee.GasPrice == nil { | ||
| return attempt, pkgerrors.New("NewEmptyTranscation: gas price cannot be nil") | ||
| return attempt, pkgerrors.New("NewEmptyTransaction failed: gas price cannot be nil. A valid gas price is required to create a transaction for force rebroadcast") |
There was a problem hiding this comment.
The error message returned from NewEmptyTxAttempt says "NewEmptyTransaction failed" which doesn’t match the actual function name. This can make debugging/searching logs harder; consider using the correct function name (or a consistent component prefix) in the message.
| return attempt, pkgerrors.New("NewEmptyTransaction failed: gas price cannot be nil. A valid gas price is required to create a transaction for force rebroadcast") | |
| return attempt, pkgerrors.New("NewEmptyTxAttempt failed: gas price cannot be nil. A valid gas price is required to create a transaction for force rebroadcast") |
pkg/txmgr/attempts.go
Outdated
| if gasFeeCap.ToInt().Cmp(Max256BitUInt) > 0 { | ||
| return pkgerrors.New("impossibly large fee cap") | ||
| return pkgerrors.New("gas fee cap validation failed: fee cap exceeds the maximum 256-bit unsigned integer value (2^256). This likely indicates a configuration error in gas price settings") | ||
| } | ||
| if gasTipCap.ToInt().Cmp(Max256BitUInt) > 0 { | ||
| return pkgerrors.New("impossibly large tip cap") | ||
| return pkgerrors.New("gas tip cap validation failed: tip cap exceeds the maximum 256-bit unsigned integer value (2^256). This likely indicates a configuration error in gas price settings") |
There was a problem hiding this comment.
The uint256 upper-bound check appears off by one: Max256BitUInt is defined as 2^256, and the code only errors when feeCap > Max256BitUInt, allowing exactly 2^256. Since uint256 max is 2^256-1, this should reject values >= 2^256 (or redefine the max constant to 2^256-1) to avoid constructing an invalid EIP-1559 tx.
1 participant
Changes: RPC client errors, simulated backend, tx store, attempts, finalizer, transmitchecker, stuck_tx_detector, contract read errors (13 files total)
Dependencies: None
See CRE Error Improvements