fix: replace SuggestGasPrice with custom SuggestedFeeAndTip #5144
Conversation
gacevicljubisa
changed the title
janos
left a comment
janos
left a comment
There was a problem hiding this comment.
I have only one comment related to inner-dependencies.
pkg/transaction/wrapped/wrapped.go
Outdated
|
|
||
| var gasFeeCap *big.Int | ||
|
|
||
| if gasPrice == nil { |
There was a problem hiding this comment.
Yes, when the Gas-Price header is used and explicitly set to 0.
This would effectively create a transaction with zero gas fees, which would likely fail when submitted to the network since miners require some compensation for processing transactions.
pkg/transaction/transaction.go
Outdated
| DefaultTipBoostPercent = 25 | ||
| MinimumGasTipCap = 1_500_000_000 // 1.5 Gwei | ||
| RedistributionTipBoostPercent = 50 | ||
| MinimumGasTipCap = 1_500_000_000 // 1.5 Gwei |
There was a problem hiding this comment.
this is really arbitrary,I would make it configurable too with defaulting only to the suggestions.
There was a problem hiding this comment.
To have it as a additiional config flag, but default should be suggested gas tip from eth_maxPriorityFeePerGas?
|
Is there a way we can manually test this? How did you tested it? |
You can execute any transaction manually, for example create a postage batch. |
| } | ||
|
|
||
| func (b *wrappedBackend) Close() error { | ||
| func (b *wrappedBackend) Close() { |
There was a problem hiding this comment.
just my 2 cents,
changing this does not seem necessary - even though ethclient.Client implements the Close() method like this.
I think it is better to use Bee common interfaces for similar functionalities such as Close() and if it does not return error (atm) then just return nil as it was before.
by that the code seems more consistent.
There was a problem hiding this comment.
Constructor wrapped.NewBackend needs to accept ethclient.Client, which implements Close(), so it was easier to remove error.
| const loggerName = "redistributionContract" | ||
| const ( | ||
| loggerName = "redistributionContract" | ||
| BoostTipPercent = 50 |
There was a problem hiding this comment.
in a future PR, this could be dynamic based on how far the node is from the end of the round phase. wdyt?
There was a problem hiding this comment.
That is nice idea
Checklist
Description
SuggestGasPricefrom codebase and replaces it with logic used insuggestedFeeAndTipmethod.minimum-gas-tip-capflag, minimum gas tip cap in wei for transactions, default 0 means use suggested gas tip cap.SuggestedFeeAndTipin wrapped package.Gas-Priceheader, that that value is used for the gas fee.wrappedpackageOpen API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):