Remove GuardianModule contract#137
Conversation
| /** | ||
| * @notice Starts the validator | ||
| */ | ||
| function callStake(bytes calldata pubKey, bytes calldata signature, bytes32 depositDataRoot) |
There was a problem hiding this comment.
Actually we are not using the function anymore, but we could keep it in the code so avoid re-deploying this contract. We can simply not call it
There was a problem hiding this comment.
we can add @deprecated as natspec for this since we won't be calling it
There was a problem hiding this comment.
Done (@deprecated isn't a valid label for functions apparently, so used a @dev)
|
how are we planning to store the EJECTION_THRESHOLD ? this is an important value from GuardianModule |
| */ | ||
| uint256 internal constant _BLS_PUB_KEY_LENGTH = 48; | ||
|
|
||
| function _validateBatchWithdrawals( |
There was a problem hiding this comment.
we are verifying/validating the paymaster signature here, but the provision node is missing that.
we shall do the similar validation of signature only for provision too, similar to what we have in GuardianModule.
or have an external function itself in this library that can be called by our off-chain service to validate the call before making actual call
There was a problem hiding this comment.
same for skipProvisioning
There was a problem hiding this comment.
I thought about this. And actually, it cannot be the paymaster that's going to call these functions, it's a multisig, so we can't use a standard signature. Since we're going to execute the transactions manually and do manual verification locally, we can skip signature verification.
I will adapt the natspec to reflect that only the ROLE_ID_NODE_PROVISIONER can call these functions (not the paymaster)
| /** | ||
| * @inheritdoc IValidatorTicket | ||
| */ | ||
| address payable public immutable override GUARDIAN_MODULE; |
There was a problem hiding this comment.
This shouldn't be immutable! Since this can be EOA and in case SK is leaked, there would be no way to change it here if this is constant.
Let's add setter/getter for this
In the current deployment, this value is not read onchain anywhere, so I figured we could just store it offchain |
2 participants
This PR removes the GuardianModule contract, related interfaces and contracts, and adapts the rest of contracts, scripts and tests.
The useful logic from the contract has been moved to PufferProtocol or the the Library LibSignatureVerifier