coverage(PrizePool): increase coverage percentage #128
Conversation
| /** | ||
| * @notice Read internal Ticket accounted balance. | ||
| */ | ||
| function getAccountedBalance() external view returns (uint256); |
There was a problem hiding this comment.
You can specify the returned value in the natspec doc for all these getters.
contracts/prize-pool/PrizePool.sol
Outdated
| @@ -83,7 +78,10 @@ abstract contract PrizePool is IPrizePool, Ownable, ReentrancyGuard, IERC721Rece | |||
| function awardBalance() external override view returns (uint256) { | |||
| return _currentAwardBalance; | |||
| } | |||
|
|
|||
| /// @inheritdoc IPrizePool | |||
| function accountedBalance() external override view returns (uint256) { | |||
There was a problem hiding this comment.
Why keep accountedBalance it we replace it by getAccountedBalance?
There was a problem hiding this comment.
Wasn't sure which one we wanted to keep. Removed accountedBalance.
contracts/prize-pool/PrizePool.sol
Outdated
| @@ -264,7 +278,7 @@ abstract contract PrizePool is IPrizePool, Ownable, ReentrancyGuard, IERC721Rece | |||
| } | |||
|
|
|||
| /// @inheritdoc IERC721Receiver | |||
| function onERC721Received(address,address,uint256,bytes calldata) external pure override returns (bytes4) { | |||
| function onERC721Received(address,address,uint256,bytes calldata) public pure override returns (bytes4) { | |||
There was a problem hiding this comment.
Not sure you would need to add public if you were only calling onERC721Received in the harness contract.
Why call it with super?
There was a problem hiding this comment.
Chuuu right @PierrickGT - removed public and just call directly.
| const NFT_TOKEN_ID = 1; | ||
|
|
||
| describe('PrizePool', function () { | ||
| let contractsOwner: SignerWithAddress; | ||
| let wallet1: SignerWithAddress; |
There was a problem hiding this comment.
Why replace by wallet1? Makes more sense to me when it's called contractsOwner.
There was a problem hiding this comment.
We use wallet1 in all the other tests.
Yes! |
contracts/prize-pool/PrizePool.sol
Outdated
| // /// @inheritdoc IPrizePool | ||
| // function accountedBalance() external override view returns (uint256) { | ||
| // return _ticketTotalSupply(); | ||
| // } |
contracts/test/PrizePoolHarness.sol
Outdated
| // function mockOnERC721Received(bytes calldata sig) external pure returns (bytes4) { | ||
| // return super.onERC721Received(address(0), address(0), 0, sig); | ||
| // } |
Co-authored-by: Pierrick Turelier <pierrick@turelier.com>
Co-authored-by: Pierrick Turelier <pierrick@turelier.com>
Added getters for public variables.
@asselstine Should we update he public access for ticket, prizeStrategy, balance and liquidityCap, so they're only accessible through the
getfunctions?