Feat/fee logic #969
Feat/fee logic #969
Conversation
…using the optional rlp tag
… not enough maxFee
paologalligit
force-pushed
the
feat/fee-logic
branch
from
59565b5 to
b103881
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/galactica #969 +/- ##
====================================================
Coverage ? 61.29%
====================================================
Files ? 226
Lines ? 24075
Branches ? 0
====================================================
Hits ? 14756
Misses ? 8136
Partials ? 1183 ☔ View full report in Codecov by Sentry. |
test/testchain/chain.go
Outdated
| forkConfig := thor.NoFork | ||
| forkConfig.VIP191 = 1 | ||
| forkConfig.BLOCKLIST = 0 | ||
| forkConfig.VIP214 = 2 |
There was a problem hiding this comment.
Is having VIP24 on block2 something we need to address ?
There was a problem hiding this comment.
This is the result of a refactoring: this function is called by already existing tests with this fork config. Actually, I might remove this function and move this fork config inside the test, passing it to the new function
| @@ -121,6 +124,11 @@ func (p *Packer) Schedule(parent *chain.BlockSummary, nowTimestamp uint64) (flow | |||
| } | |||
| } | |||
|
|
|||
| var baseFee *big.Int | |||
There was a problem hiding this comment.
Would it be worth adding a log saying that we've entered galactica ?
There was a problem hiding this comment.
Yes, sounds like a good idea
packer/packer_test.go
Outdated
| @@ -113,7 +113,6 @@ func TestP(t *testing.T) { | |||
|
|
|||
| best := repo.BestBlockSummary() | |||
| fmt.Println(best.Header.Number(), best.Header.GasUsed()) | |||
There was a problem hiding this comment.
Might want to change this print to log ?
There was a problem hiding this comment.
Actually I had no idea of this line, it looks like some leftover print statement from couple of years ago. I'll replace it with some assert on the best block
| if tx.MaxFeePerGas() == nil { | ||
| return nil, errors.New("max fee per gas is required") | ||
| } | ||
| if tx.MaxPriorityFeePerGas() == nil { | ||
| return nil, errors.New("max priority fee per gas is required") |
There was a problem hiding this comment.
This will be set elsewhere, even it its a legacy tx correct ?
There was a problem hiding this comment.
Yes, this is to make sure we have not nil pointer dereference. For the legacy it's impossible this scenario since they use the gasPriceCoef in combination with the old GasPrice func, and gasPriceCoef cannot be nil
paologalligit
force-pushed
the
feat/fee-logic
branch
from
72f9340 to
6205c3b
Compare
paologalligit
force-pushed
the
feat/fee-logic
branch
from
e082285 to
50fb433
Compare
Description
This PR introduces the logic to calculate the fees dynamically based on the VIP-Dynamic Fee Market.
GHIssue
Type of change
Please delete options that are not relevant.
Checklist: