Generalized upgrades #426
Generalized upgrades #426
Conversation
Co-authored-by: aaronbuchwald <[email protected]>
Co-authored-by: aaronbuchwald <[email protected]>
Co-authored-by: aaronbuchwald <[email protected]>
| // IsPrecompileEnabled returns whether precompile with [address] is enabled at [blockTimestamp]. | ||
| func (c *ChainConfig) IsPrecompileEnabled(address common.Address, blockTimestamp *big.Int) bool { | ||
| config := c.GetPrecompileConfig(address, blockTimestamp) | ||
| func (c *ChainConfig) IsPrecompileEnabled(name string, blockTimestamp *big.Int) bool { |
There was a problem hiding this comment.
could we update the comment to match?
Also should we call this key or rename module.Key() -> module.Name()
There was a problem hiding this comment.
Initially I named it as Name but then it sounds a bit like human-readable name such as TxAllowList. but instead this serves as a json map key as txAllowList and IMO Key fits better.
| return err | ||
| } | ||
| if len(raw) != 1 { | ||
| return fmt.Errorf("PrecompileUpgrade must have exactly one key") |
There was a problem hiding this comment.
nit: errors.New if no formatting is included in the error
| return configs[len(configs)-1] // return the most recent config | ||
| } | ||
|
|
||
| // getActivatingPrecompileConfigs returns all forks configured to activate during the state transition from a block with timestamp [from] |
There was a problem hiding this comment.
| // getActivatingPrecompileConfigs returns all forks configured to activate during the state transition from a block with timestamp [from] | |
| // getActivatingPrecompileConfigs returns all upgrades configured to activate during the state transition from a block with timestamp [from] |
| if val := c.getActivePrecompileConfig(key, blockTimestamp); val != nil { | ||
| return val | ||
| } | ||
| return nil |
There was a problem hiding this comment.
could we return c.getActivePrecompileConfig(key, blockTimestamp)?
| PrecompileUpgrades: []params.PrecompileUpgrade{ | ||
| { | ||
| TxAllowListConfig: txallowlist.NewTxAllowListConfig(big.NewInt(enableAllowListTimestamp.Unix()), testEthAddrs[0:1], nil), | ||
| Config: txallowlist.NewTxAllowListConfig(big.NewInt(enableAllowListTimestamp.Unix()), testEthAddrs[0:1], nil), |
There was a problem hiding this comment.
nit: should we remove Config: for consistency?
| return err | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
can we add a comment that explains this pattern?
| New() StatefulPrecompileConfig | ||
| } | ||
|
|
||
| // StatefulPrecompileConfig defines the interface for a stateful precompile to |
There was a problem hiding this comment.
could we complete the sentence? seems it was left half way before
|
Closing in favor of #434 |
Why this should be merged
How this works
How this was tested