SendTx contradicting responses should log all fatal error responses too in the critical logs #13097
Conversation
…oo in the critical logs
|
| @@ -11,6 +11,7 @@ import ( | |||
|
|
|||
| "github.com/prometheus/client_golang/prometheus" | |||
| "github.com/prometheus/client_golang/prometheus/promauto" | |||
| "go.uber.org/multierr" | |||
There was a problem hiding this comment.
I think we're trying to move away from multierr and use https://pkg.go.dev/errors#Join instead (similar discussion here)
| @@ -661,21 +662,25 @@ func (c *multiNode[CHAIN_ID, SEQ, ADDR, BLOCK_HASH, TX, TX_HASH, EVENT, EVENT_OP | |||
| func aggregateTxResults(resultsByCode sendTxErrors) (txResult error, err error) { | |||
| severeErrors, hasSevereErrors := findFirstIn(resultsByCode, sendTxSevereErrors) | |||
| successResults, hasSuccess := findFirstIn(resultsByCode, sendTxSuccessfulCodes) | |||
| var severeErrorsList error | |||
| for _, severeError := range severeErrors { | |||
| severeErrorsList = multierr.Append(severeErrorsList, severeError) | |||
There was a problem hiding this comment.
severeErrors only contains errors of single sever sub type (only Fatal or only Underpriced, etc).
Aren't we want to collect all the errors?
Also we already log all the errors here
| } | ||
|
|
||
| // other errors are temporary - we are safe to return success | ||
| return successResults[0], nil | ||
| } | ||
|
|
||
| if hasSevereErrors { | ||
| return severeErrors[0], nil | ||
| return severeErrorsList, nil |
There was a problem hiding this comment.
Won't this affect errors parsing in the TXM?
Example: we use following (: |^)gas price too low$ regex to classify Underpriced error.
If two RPCs return gas price too low error, combined version will look like gas price too low; gas price too low,
which no longer match the regex.
| // return success, since at least 1 node has accepted our broadcasted Tx, and thus it can now be included onchain | ||
| return successResults[0], fmt.Errorf(errMsg) | ||
| return successResults[0], multierr.Combine(fmt.Errorf(errMsg), severeErrorsList) |
There was a problem hiding this comment.
Should not we wrap severeErrorsList instead of combining? In that case instead of getting
found contradictions in nodes replies on SendTransaction: got success and severe error(s): ; fatal; fatal, we'll get
found contradictions in nodes replies on SendTransaction: got success and severe error(s): fatal; fatal
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
Collect all fatal errors and send them back in status, and log them.