eth: Fix error logging for failed replacement tx #1977
Conversation
yondonfu
force-pushed
the
yf/fix-replacement-log
branch
from
947c73f
to
14e95a4
Compare
| @@ -237,6 +237,7 @@ func TestTransactionManager_Replace(t *testing.T) { | |||
| logsAfter = glog.Stats.Info.Lines() | |||
| assert.Nil(err) | |||
| expTx := types.NewTransaction(1, *stubTx.To(), stubTx.Value(), 100000, calcReplacementGasPrice(stubTx), stubTx.Data()) | |||
| assert.Equal(tx.Hash(), expTx.Hash()) | |||
There was a problem hiding this comment.
Fun discovery that led to the addition of this assertion as well as the assertion on L253:
The go-ethereum Transaction type maintains a cache for the hash of the tx and the Hash() method will only compute the hash of the tx if the cache is not populated. The internal hash field of the struct is the zero value until Hash() is called for the first time.
Prior to this PR, newSignedTx.Hash() was not called in replace() when an error was not returned from SubmitTransaction(). But, now newSignedTx.Hash() is called in replace() when an error is not returned from SubmitTransaction() because we call newSignedTx.Hash() in the log statement on L188. As a result, without these assertions that call the Hash() method on both tx and expTx, one of the structs would have the zero value for the internal hash field and the assertion comparing tx and expTx would fail. By first calling Hash() on both tx and expTx we ensure that the internal hash field of both structs is populated.
There was a problem hiding this comment.
Oh nice find, interesting behaviour.
What does this pull request do? Explain your changes. (required)
This PR logs the correct error when a replacement tx fails at submission time.
Specific updates (required)
See commit history.
How did you test each of these updates (required)
N/A
Does this pull request close any open issues?
Related to the logs provided in #1964.
Checklist:
makeruns successfully./test.shpass