BeefyClient Improvements #1165
BeefyClient Improvements #1165
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1165 +/- ##
==========================================
+ Coverage 77.83% 78.55% +0.72%
==========================================
Files 14 14
Lines 415 429 +14
Branches 76 77 +1
==========================================
+ Hits 323 337 +14
Misses 75 75
Partials 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -9,6 +9,7 @@ import {Uint16Array, createUint16Array} from "./utils/Uint16Array.sol"; | |||
| import {Math} from "./utils/Math.sol"; | |||
| import {MMRProof} from "./utils/MMRProof.sol"; | |||
| import {ScaleCodec} from "./utils/ScaleCodec.sol"; | |||
| import "forge-std/console.sol"; | |||
There was a problem hiding this comment.
This import is probably not needed.
| @@ -289,6 +302,8 @@ contract BeefyClient { | |||
| prevRandao: 0, | |||
| bitfieldHash: keccak256(abi.encodePacked(bitfield)) | |||
| }); | |||
|
|
|||
| emit NewTicket(msg.sender, commitment.blockNumber); | |||
There was a problem hiding this comment.
We should add the ticketId to NewTicket and NewMMRRoot events to be able to link them.
| /// @dev Renounce all administrative privileges | ||
| function renounce() external { | ||
| delete forceCheckpointOperator; | ||
| } |
There was a problem hiding this comment.
Not sure fully understand why we provide an external renounce here.
I would assume in the interim period we just make BeefyClient inherited from
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable.sol for access control and after production and stable for a while we remove the owner control with an upgrade.
|
Another concern is do we still wanna make BEEFY_CLIENT immutable? |
This is by design, the beefy client and agent executor are so critically important that they should only be changed via a contract upgrade of Gateway.sol. It also reduces gas costs when referencing those contracts. |
Adds two important & necessary improvements:
The ability for a trusted operator to set an initial checkpoint after the contract has already been deployed. This is necessary so that we can parallelize the deployment of the bridge. We need to deploy the contracts next week, however BEEFY is only being activated on Polkadot at the end of April
The
submitInitialstep emits aNewTicket(relayer, beefyBlockNumber)event after completing successfully. As discussed with W3F in yesterday's call, this allows relayers to know that there is an ongoing interactive submission, and that they should not waste gas trying to submit themselves.Resolve: SNO-949