Skip to content

upgrade consensus contracts to solidity 0.8.30#189

Open
Andrew-Pohl wants to merge 1 commit into
masterfrom
upgrade-solidity
Open

upgrade consensus contracts to solidity 0.8.30#189
Andrew-Pohl wants to merge 1 commit into
masterfrom
upgrade-solidity

Conversation

@Andrew-Pohl

Copy link
Copy Markdown
Member

Summary

Upgrades all consensus contracts (Consensus, BlockReward, Voting, ProxyStorage, EternalStorage/Proxy + abstracts, interfaces and test mocks) from Solidity 0.4.24 to 0.8.30.

These are drop-in implementation upgrades: storage layout and all keccak storage keys are unchanged, so the new implementations can be rolled out to the live proxies through the existing ballot → ProxyStorage.setContractAddressupgradeTo governance flow. No re-initialization is required (isInitialized remains set in proxy storage).

Compiler / EVM target

  • solc 0.8.30, optimizer unchanged (enabled, 200 runs)
  • evmVersion is explicitly pinned to berlin. Per config/spec.json, Fuse/Spark have Constantinople + Istanbul/Berlin EIPs activated but not Shanghai, so default 0.8.20+ codegen would emit PUSH0 and produce bytecode that cannot run on chain.

Contract changes

  • SafeMath/Math removedopenzeppelin-solidity dependency dropped entirely. .add/.sub/.mul/.div/.mod replaced with native operators (0.8 checked arithmetic has the same revert-on-overflow semantics). Math.min/max replaced with internal _min/_max helpers in ConsensusUtils.
  • Fallback split (behaviour preserved):
    • Consensus: receive() + fallback() both stake msg.value for msg.sender, matching the old catch-all fallback (plain send = stake, unknown selector = stake).
    • EternalStorageProxy: receive() + fallback() both delegatecall to the implementation via a shared _delegateToImplementation().
  • Mechanical 0.8 migration: explicit memory/calldata data locations, arr.length--arr.pop(), payable(...) casts for .transfer() and proxy address conversions, abstract/virtual/override wiring (including the onlyConsensus modifier override in VotingMock), 0.8 assembly syntax (gas(), calldatasize(), ...), SPDX identifiers.
  • Removed two tautological require(x >= 0) checks on uints in BlockReward (always true; compiler warnings only).

Behaviour notes

  • DECIMALS - validatorFee() in delegator reward distribution was an unchecked subtraction in 0.4; it now reverts on underflow. Unreachable in practice (fee capped at 100%), but a strictly safer failure mode.
  • Arithmetic overflow/underflow now reverts with Panic(0x11) instead of a silent wrap / plain revert. All existing guards (explicit requires) still fire first on the normal paths.

Tooling

  • hardhat pinned to ~2.22.19 (last 2.x line fully supporting Node 18; newer requires Node 20+)
  • Removed unused solc@0.4.24 devDependency (flatten uses hardhat flatten)
  • yarn.lock regenerated

Testing

  • yarn compile — 20 files compile cleanly (evm target: berlin)
  • yarn test173 passing, 0 failing, including the upgradeTo suites that exercise upgrading proxies in place and verifying state survives


function upgradeConsensusMock(address _implementation) public {
EternalStorageProxy(getConsensus()).upgradeTo(_implementation);
EternalStorageProxy(payable(getConsensus())).upgradeTo(_implementation);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to add payable here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre 0.5.0 all addresses were payable. in 0.5.0 it split to non payable to payable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants