cleanup: shorten upgrade names #605
Conversation
| @@ -515,30 +437,30 @@ type ChainConfig struct { | |||
| MuirGlacierBlock *big.Int `json:"muirGlacierBlock,omitempty"` // Eip-2384 (bomb delay) switch block (nil = no fork, 0 = already activated) | |||
|
|
|||
| // Avalanche Network Upgrades | |||
| ApricotPhase1BlockTimestamp *uint64 `json:"apricotPhase1BlockTimestamp,omitempty"` // Apricot Phase 1 Block Timestamp (nil = no fork, 0 = already activated) | |||
| ApricotPhase1Time *uint64 `json:"apricotPhase1Time,omitempty"` // Apricot Phase 1 Block Timestamp (nil = no fork, 0 = already activated) | |||
There was a problem hiding this comment.
This is the only bit that concerns me: presumably there will be JSON somewhere that is broken by this? I see that you updated some tests, but what needs to change in production?
There was a problem hiding this comment.
This is mostly fine since we have never used them from a JSON file. However it's concerning that we might have stored this as a part of genesis and we do check them here. If I'm not mistaken this might be broken since the storedcfg will have nil timestamps.
There was a problem hiding this comment.
what is the best way to test this? is running a node from scratch sufficient?
should we have some node then apply this patch then restart it?
There was a problem hiding this comment.
yes we should have a node already stored the genesis to test this out.
|
In subnet-evm, we have: What do you recommend for consistency here? Notably we have |
I'd say |
|
After looking at Subnet-EVM code, I think it's probably best to go with |
|
going to move this to draft for now until I fix it up. |
Why this should be merged
I think "UpgradeTime" is sufficiently expressive and shorter than "UpgradeBlockTimestamp"
How this works
Renames things, should not impact deployed chains since these values are specified by the avalanchego times not the genesis
How this was tested
CI