jkilpatr
commented
2 years ago Good call on the struct for signature members, not that much more difficult to add now that we have structs as arguments in general.
The signature verification suggestion here may put this into the running as a duplicate of the transaction malleability reports
loudoguno
Handle
pauliax
Vulnerability details
Impact
Style issues that you may want to apply or reject, no impact on security. Grouping them together as one submission to reduce waste. Consider fixing or ignoring them, up to you.
This comment should be updated to also include rewardAmount and rewardToken: // The format of the checkpoint is: // h(gravityId, "checkpoint", valsetNonce, validators[], powers[])
Extract contanst variables, this will make the code more readable and maintanable. Examples of such variables are: // bytes32 encoding of the string "checkpoint" bytes32 methodName = 0x636865636b706f696e7400000000000000000000000000000000000000000000 // bytes32 encoding of "transactionBatch" 0x7472616e73616374696f6e426174636800000000000000000000000000000000 // bytes32 encoding of "logicCall" 0x6c6f67696343616c6c0000000000000000000000000000000000000000000000
v, r and s are always present in the signature. Consider extracting them to a struct to group these variables together: uint8[] memory _v, bytes32[] memory _r, bytes32[] memory _s,
As you are using other OpenZeppelin contracts, I think you should also consider re-using their ECDSA library for signature verifications: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol