Supplement baseFee and gasLimit for pre gingerbread blocks #302
Conversation
…rbreadBlockBaseFee"
There was a problem hiding this comment.
Hi @Kourin1996, looks good overall, but I don't see a reason to store the gas limit in the database, when we have it hardcoded. So I would just store the base fee.
Also how did you get your benchmark results? https://github.com/celo-org/celo-blockchain-planning/issues/384#issuecomment-2553929355
core/rawdb/celo_accessors_test.go
Outdated
| t.Run(test.name, func(t *testing.T) { | ||
| for _, seed := range test.seedData { | ||
| err := db.Put(preGingerbreadAdditionalFieldsKey(seed.hash), seed.data) | ||
| assert.NoError(t, err) |
There was a problem hiding this comment.
I would use require here instead of assert. A failed assert statement does not abort the test, whereas require will. In this case if the insertion fails then there is no reason to continue the test.
There was a problem hiding this comment.
Learned new best practice! Thank you :) I will fix that
There was a problem hiding this comment.
Most of assert.* have been replaced with require.*. Resolved the following related comments.
internal/ethapi/celo_block.go
Outdated
| } | ||
|
|
||
| // retrievePreGingerbreadGasLimit retrieves a gas limit at given height from hardcoded values | ||
| func retrievePreGingerbreadGasLimit(backend CeloBackend, height *big.Int) *big.Int { |
There was a problem hiding this comment.
Seems like this could take a chain config instance instead of the CeloBackend
There was a problem hiding this comment.
Yes, I modified codes so that only base fee will be stored in DB at commit a67a0ad
Set up a local cluster using |
…lds-in-rpc-pre-gingerbread-block
|
Question regarding the general approach: we could also do those calculations during the migration right. That would trade some complexity there for simpler code in the codebase. Any opinions on that? |
The problem with that approach is that we are not syncing this data, so if we did that, snap sync nodes would not have it. So I think it's better to calculate it on demand as it is done here. |
Closes https://github.com/celo-org/celo-blockchain-planning/issues/384
This PR adds
baseFeePerGasandgasLimitto the response of pre Gingerbread block in JSON-RPC