Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

100% Code Coverage on Signer Launchpad #411

Merged
merged 2 commits into from
May 13, 2024
Merged

Conversation

nlordell
Copy link
Collaborator

@nlordell nlordell commented May 6, 2024

This PR adds 100% code coverage as unit test for the Safe signer launchpad contract.

----------------------------------|----------|----------|----------|----------|----------------|
File                              |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------------------------------|----------|----------|----------|----------|----------------|
 contracts/4337/                  |      100 |      100 |      100 |      100 |                |
  SafeSignerLaunchpad.sol         |      100 |      100 |      100 |      100 |                |
----------------------------------|----------|----------|----------|----------|----------------|

This was introduced as one large PR as I don't see much value in splitting this into individual PRs for review (all changes are related to the introduced unit tests).

@nlordell nlordell requested a review from a team as a code owner May 6, 2024 08:41
@nlordell nlordell requested review from akshay-ap, mmv08 and remedcu and removed request for a team May 6, 2024 08:41
@@ -303,6 +310,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage {
// See <https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require>.
mstore(0x00, hex"4e487b71")
mstore(0x04, 0x21)
revert(0, 0x24)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug caught by the unit tests.

@@ -25,15 +25,15 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage {
* @notice The EIP-712 type-hash for the domain separator used for verifying Safe initialization signatures.
* @custom:computed-as keccak256("EIP712Domain(uint256 chainId,address verifyingContract)")
*/
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218;
bytes32 private constant _DOMAIN_SEPARATOR_TYPEHASH = 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent naming.

@nlordell nlordell force-pushed the 308/single-setup-unit-tests branch from 95cb459 to 89a0cb2 Compare May 10, 2024 11:07
Base automatically changed from 308/single-setup to main May 10, 2024 11:09
@coveralls
Copy link

coveralls commented May 10, 2024

Pull Request Test Coverage Report for Build 9061845137

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+13.7%) to 100.0%

Totals Coverage Status
Change from base Build 9031345321: 13.7%
Covered Lines: 132
Relevant Lines: 132

💛 - Coveralls

This PR adds 100% code coverage as unit test for the Safe signer
launchpad contract.
@nlordell nlordell force-pushed the 308/single-setup-unit-tests branch 2 times, most recently from bec13b0 to 8a7eab4 Compare May 13, 2024 11:15
@nlordell nlordell merged commit d45551d into main May 13, 2024
12 checks passed
@nlordell nlordell deleted the 308/single-setup-unit-tests branch May 13, 2024 11:48
@github-actions github-actions bot locked and limited conversation to collaborators May 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants