R4R: system contract upgrade framework and upgrade tokenhub#16
R4R: system contract upgrade framework and upgrade tokenhub#16
Conversation
8bffb98 to
c8a05e1
Compare
| /* testnet upgrade config */ | ||
|
|
||
| // setup testnet upgrade config | ||
| devnetUpgrapdeConfig[20000] = Upgrade{ |
There was a problem hiding this comment.
can not just simple define devnetUpgrapdeConfig[20000] here. Because 20000 is treat as fork height here, we must define the fork height on chainConfig, like dao fork get DAOForkBlock in config. Otherwise will break the handshake protocol about forkId.
There was a problem hiding this comment.
We need define a map like map[forkname] []*UpgradeConfig here. Then define different height for the fork for different network on Params file
c8a05e1 to
9669e2a
Compare
911e1fe to
45a9e2c
Compare
f10d500 to
70a26aa
Compare
|
|
||
| var ( | ||
| // genesis contracts | ||
| validatorContract = common.HexToAddress("0x0000000000000000000000000000000000001000") |
There was a problem hiding this comment.
we have redefined the system contract address here, since parlia have done it too. But I don't know where to put it better either. Just a bit uncomfortable for this.
There was a problem hiding this comment.
System addresses in parlia.go are not completed. Besides, I think it would be better to defind all addresses in here ranther than in consensus engine.
| /* Add mainnet genesis hash */ | ||
| case params.ChapelGenesisHash: | ||
| network = chapel | ||
| case params.RialtoGenesisHash: |
There was a problem hiding this comment.
if no match with any network, just return
There was a problem hiding this comment.
In the further development, we might need to start a local network. This local network might also need system upgrade, so I added a default network type.
|
|
||
| logger.Info(fmt.Sprintf("Apply upgrade %s at height %d", upgrade.UpgradeName, blockNumber.Int64())) | ||
| for _, cfg := range upgrade.Configs { | ||
| logger.Info(fmt.Sprintf("Upgrade contract %s to commit %s", cfg.ContractAddr.String(), cfg.CommitUrl)) |
There was a problem hiding this comment.
==> logger.Info("Upgrade contract to commit ", "address", cfg.ContractAddr.String(),"commit url", cfg.CommitUrl)
There was a problem hiding this comment.
commit url contains space. Besides, I think no need to follow this style. In many case, logger.Info is used together with fmt.Sprintf.
| return | ||
| } | ||
|
|
||
| logger.Info(fmt.Sprintf("Apply upgrade %s at height %d", upgrade.UpgradeName, blockNumber.Int64())) |
There was a problem hiding this comment.
The log style here violate how the log being used.
| Configs []*UpgradeConfig | ||
| } | ||
|
|
||
| type beforeUpgrade func(blockNumber *big.Int, contractAddr common.Address, statedb *state.StateDB) |
There was a problem hiding this comment.
there is no need to define type beforeUpgrade and afterUpgrade, upgradeHook is enough.
There was a problem hiding this comment.
please add return (error) to this hook.
There was a problem hiding this comment.
there is no need to define type
beforeUpgradeandafterUpgrade,upgradeHookis enough.
I don't think upgradeHook is enough. Sometime we have to do cleanup work before update code, and sometime we have to do init work after update code
There was a problem hiding this comment.
Sorry for misunderstand. We can just define one function typeupgradeHook but multiple instances
type UpgradeConfig struct {
BeforeUpgrade ugradeHook
AfterUpgrade ugradeHook
...
}
549f073 to
32ec9be
Compare
8712a64 to
11a85ae
Compare
11a85ae to
650d066
Compare
[R4R] minor fix for ramanujan upgrade
[R4R]update chapel ramanujan fork
Co-authored-by: Welkin <welkin.b@nodereal.com>
Co-authored-by: galaio <galaio@users.noreply.github.com>
…16) * fix: lint error * fix: ut that need reset cfg status * fix: evmRun test * fix: clear cfg cache for new file * feat: add more mir tests and fix jumpi issues * fix: more mir tests pass * fix: CALL and RETURNDATA gas issue * feat: patch test updates to rebased branch * feat: more patch * feat: modify tests after rebase * feat: progress * feat: more progress on tests * feat: more test coverages * fix: more fixes for all uts to pass * feat: restore tests/testdata
No description provided.