core: respect price bump threshold#15401
Conversation
karalabe
left a comment
karalabe
left a comment
There was a problem hiding this comment.
Thanks for this, generally good, just keep the original coverage of the tests :)
| t.Fatalf("original queued transaction replacement error mismatch: have %v, want %v", err, ErrReplaceUnderpriced) | ||
| } | ||
| if err := pool.AddRemote(pricedTransaction(2, big.NewInt(100000), big.NewInt(threshold+1), key)); err != nil { | ||
| if err := pool.AddRemote(pricedTransaction(2, big.NewInt(100000), big.NewInt(threshold), key)); err != nil { |
There was a problem hiding this comment.
Please retain both checks to ensure correct behavior on both sides of the boundary (i.e. just rejected vs. just accepted). I.e. in your code threshold - 1 should fail, but threshold should succeed.
| t.Fatalf("original proper pending transaction replacement error mismatch: have %v, want %v", err, ErrReplaceUnderpriced) | ||
| } | ||
| if err := pool.AddRemote(pricedTransaction(0, big.NewInt(100000), big.NewInt(threshold+1), key)); err != nil { | ||
| if err := pool.AddRemote(pricedTransaction(0, big.NewInt(100000), big.NewInt(threshold), key)); err != nil { |
There was a problem hiding this comment.
Please retain both checks to ensure correct behavior on both sides of the boundary (i.e. just rejected vs. just accepted). I.e. in your code threshold - 1 should fail, but threshold should succeed.
karalabe
left a comment
karalabe
left a comment
There was a problem hiding this comment.
Thanks, LGTM!
I've added a followup commit to fix some issues in the test failure messages (and expand them a bit). Will squash when CI is green.
* core: allow price bump at threshold * core: test changes to allow price bump at threshold * core: reinstate tx replacement test underneath threshold * core: minor test failure message cleanups
2 participants
According to the usage information the transaction price bump value is defined as "Price bump percentage to replace an already existing transaction". However a replacement value at the price bump fails to replace the transaction.
To give an example: if a transaction is sent with a gas price of 1GWei then given the definition of price bump and the default value of 10(%) it would be expected that a replacement gas price of 1.1GWei would result in a successful replacement. This patch alters the code so that rather than reject this as underpriced the transaction is accepted.
The patch ensures that low gas prices are handled correctly by ensuring that the replacement gas price is absolutely larger than the original gas price as well as above the threshold.