-
Notifications
You must be signed in to change notification settings - Fork 21.5k
internal/ethapi,params: add eth_config
#32239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
params/config.go
Outdated
|
|
||
| switch { | ||
| case c.IsOsaka(london, time): | ||
| next = c.OsakaTime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to return something like Maxuint64 here
25c0e49 to
756304b
Compare
|
Implementation seems correct now, working on some tests. |
|
Should be ready to go, PTAL! |
MariusVanDerWijden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, small nit
33c972b to
815a841
Compare
internal/ethapi/api.go
Outdated
| } | ||
| forkid := forkid.NewID(c, types.NewBlockWithHeader(genesis), ^uint64(0), t).Hash | ||
| return &config{ | ||
| ActivationTime: t, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Activation time is required. If a fork is activated at genesis the value 0 is used. If the fork is not scheduled to be activated or its activation time is unknown it should not be in the rpc results.
Fix like this?
diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go
index eb662f9a71..9f2e84c194 100644
--- a/internal/ethapi/api.go
+++ b/internal/ethapi/api.go
@@ -1188,9 +1188,14 @@ func (api *BlockChainAPI) Config(ctx context.Context) (*configResponse, error) {
for addr, c := range vm.ActivePrecompiledContracts(rules) {
precompiles[c.Name()] = addr
}
+ // Activation time is required. If a fork is activated at genesis the value 0 is used
+ activateTime := t
+ if genesis.Time >= t {
+ activateTime = 0
+ }
forkid := forkid.NewID(c, types.NewBlockWithHeader(genesis), ^uint64(0), t).Hash
return &config{
- ActivationTime: t,
+ ActivationTime: activateTime,
BlobSchedule: c.BlobConfig(c.LatestFork(t)),
ChainId: (*hexutil.Big)(c.ChainID),
ForkId: forkid[:],
|
One nit, otherwise lgtm |
Co-authored-by: rjl493456442 <[email protected]>
e098cb5 to
360152b
Compare
gballet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
~Will probably be mostly supplanted by ethereum#32224, but this should do for now for devnet 3.~ Seems like ethereum#32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed. --------- Co-authored-by: MariusVanDerWijden <[email protected]> Co-authored-by: Guillaume Ballet <[email protected]> Co-authored-by: rjl493456442 <[email protected]>
~Will probably be mostly supplanted by ethereum#32224, but this should do for now for devnet 3.~ Seems like ethereum#32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed. --------- Co-authored-by: MariusVanDerWijden <[email protected]> Co-authored-by: Guillaume Ballet <[email protected]> Co-authored-by: rjl493456442 <[email protected]>
Will probably be mostly supplanted by #32224, but this should do for now for devnet 3.Seems like #32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed.