Features/new view functions #441
Conversation
There was a problem hiding this comment.
@pedroargento Looking good, just some minor cosmetic changes!
@vfusco Do you need a view method for the number of submitted claims as well, or just for the accepted ones?
| } | ||
|
|
||
| /// @inheritdoc IConsensus | ||
| function numberOfClaimsAccepted() public view override returns (uint256) { |
There was a problem hiding this comment.
This function is not used internally, so it can be marked as external.
| /// the integer division of the block number by the epoch length. | ||
| function getEpochLength() external view returns (uint256); | ||
|
|
||
| function numberOfClaimsAccepted() external view returns (uint256); |
There was a problem hiding this comment.
Should be renamed as getNumberOfAcceptedClaims.
(For better wording in English, and consistency with getNumberOfInputs and contracts v3.)
Lacks documentation in NatSpec format.
|
|
||
| /// @notice Indexes accepted claims by application contract address. | ||
| mapping(address => mapping(bytes32 => bool)) private _validOutputsMerkleRoots; | ||
| uint256 _numberOfClaimsAccepted; |
There was a problem hiding this comment.
Should be renamed as _numOfAcceptedClaims.
(To keep consistency with contracts v3.)
|
|
||
| /// @notice The number of outputs executed by the application. | ||
| /// @dev See the `numberOfOutputsExecuted` function. | ||
| uint256 _numberOfOutputsExecuted; |
There was a problem hiding this comment.
Should be renamed as _numOfExecutedOutputs.
(To keep consistency with contracts v3.)
| function getDeploymentBlockNumber() external view returns (uint256); | ||
|
|
||
| /// @notice Get number of outputs executed by the application. | ||
| function numberOfOutputsExecuted() external view returns (uint256); |
There was a problem hiding this comment.
Should be renamed as getNumberOfExecutedOutputs.
(To keep consistency with contracts v3.)
| authority.submitClaim(appContract, lastProcessedBlockNumber, claim); | ||
| } | ||
|
|
||
| function testSubmitMultipleValidClaims(uint8 numberOfClaims) public { |
There was a problem hiding this comment.
You could receive an array of bytes32 values (the claims) and iterate over it, so that you don't have to generate random claims on-the-fly.
| vm.roll(blockNum); | ||
|
|
||
| for (uint256 i; i < numberOfClaims; ++i) { | ||
| blockNum = (i + 1) * epochLength - 1 + 1; |
There was a problem hiding this comment.
Can we remove the
| quorum.submitClaim(claim); | ||
| } | ||
|
|
||
| function testMultipleClaimsAcceptedCounter(uint8 numberOfClaims) external { |
There was a problem hiding this comment.
The same trick could be applied here (receive the array of claims as a fuzzy parameter).
No description provided.