Skip to content

Add missing test for OwnerFacet & LibOwnerTwoStep#215

Merged
maxnorm merged 2 commits into
Perfect-Abstractions:mainfrom
maxnorm:main
Nov 23, 2025
Merged

Add missing test for OwnerFacet & LibOwnerTwoStep#215
maxnorm merged 2 commits into
Perfect-Abstractions:mainfrom
maxnorm:main

Conversation

@maxnorm

@maxnorm maxnorm commented Nov 23, 2025

Copy link
Copy Markdown
Collaborator

Summary

Improve coverage for OwnerFacet & LibOwnerTwoStep to 100%

Changes Made

  • added direct renounceOwnership to both test suite. the logic was testing via transfer ownership to address zero but not direct call test was made
  • added test for the requireOwner view function of LibOwnerTwoStep
image

Checklist

Before submitting this PR, please ensure:

  • Code follows the Solidity feature ban - No inheritance, constructors, modifiers, public/private variables, external library functions, using for directives, or selfdestruct

  • Code follows Design Principles - Readable, uses diamond storage, favors composition over inheritance

  • Code matches the codebase style - Consistent formatting, documentation, and patterns (e.g. ERC20Facet.sol)

  • Code is formatted with forge fmt

  • Existing tests pass - Run tests to be sure existing tests pass.

  • New tests are optional - If you don't provide tests for new functionality or changes then please create a new issue so this can be assigned to someone.

  • All tests pass - Run forge test and ensure everything works

  • Documentation updated - If applicable, update relevant documentation

Make sure to follow the contributing guidelines.

Additional Notes

@netlify

netlify Bot commented Nov 23, 2025

Copy link
Copy Markdown

👷 Deploy request for compose-diamonds pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 8db0d28

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Coverage

Metric Coverage Details
Lines 78% 1480/1896 lines
Functions 81% 278/344 functions
Branches 68% 219/322 branches

Last updated: Sun, 23 Nov 2025 14:25:15 GMT for commit 8db0d28

@github-actions

Copy link
Copy Markdown
Contributor

Gas Report

Comparing gas usage between main and main

Summary

  • Optimized: 19 functions (🟢 -491 gas)
  • Increased: 28 functions (🔴 +896 gas)
  • Net Change: 🔴 +405 gas

Details

Contract Function Before After Change
Test testFuzz_SequentialTransfers() N/A N/A 🔴 +85
Test test_TransferOwnership_EmitsCorrectPreviousOwner() N/A N/A 🔴 +78
Test test_RevertWhen_TransferOwnership_CalledByNonOwner() N/A N/A 🔴 +73
Test test_TransferOwnership_ToSelf() N/A N/A 🔴 +72
Test test_RenounceOwnership_SetsOwnerToZero() N/A N/A 🔴 +71
Test testFuzz_RenouncePreventsAllTransfers() N/A N/A 🔴 +57
Test test_TransferOwnership_FromPendingOwner_Allowed() N/A N/A 🟢 -44
Test testFuzz_TransferOwnership() N/A N/A 🟢 -44
Test test_TransferOwnership_AllowsTransferToZeroAddress() N/A N/A 🟢 -44
Test test_RenounceOwnership_PreventsNewTransfersAfterForceRenounce() N/A N/A 🟢 -44
Test test_GetStorage_ReturnsCorrectOwner() N/A N/A 🟢 -44
Test test_StorageCollision_DocumentedBug() N/A N/A 🔴 +44
Test test_PendingOwner_ReturnsCurrentPendingOwner() N/A N/A 🔴 +42
Test test_CancelPendingTransfer() N/A N/A 🔴 +36
Test test_AcceptOwnership_CurrentOwnerCanCall() N/A N/A 🟢 -35
Test test_TransferOwnership_EmitsOwnershipTransferStartedEvent() N/A N/A 🔴 +23
Test test_RenounceOwnership_CannotBeCompleted() N/A N/A 🔴 +23
Test test_StorageSlot_CurrentlyCollides() N/A N/A 🔴 +23
Test test_TransferOwnership_CanUpdatePendingOwner() N/A N/A 🟢 -22
Test test_GetStorage_ReturnsCorrectPendingOwner() N/A N/A 🟢 -22

Showing top 20 changes out of 47 total.

View all 47 changes
Contract Function Before After Change
Test testFuzz_SequentialTransfers() N/A N/A 🔴 +85
Test test_TransferOwnership_EmitsCorrectPreviousOwner() N/A N/A 🔴 +78
Test test_RevertWhen_TransferOwnership_CalledByNonOwner() N/A N/A 🔴 +73
Test test_TransferOwnership_ToSelf() N/A N/A 🔴 +72
Test test_RenounceOwnership_SetsOwnerToZero() N/A N/A 🔴 +71
Test testFuzz_RenouncePreventsAllTransfers() N/A N/A 🔴 +57
Test test_TransferOwnership_FromPendingOwner_Allowed() N/A N/A 🟢 -44
Test testFuzz_TransferOwnership() N/A N/A 🟢 -44
Test test_TransferOwnership_AllowsTransferToZeroAddress() N/A N/A 🟢 -44
Test test_RenounceOwnership_PreventsNewTransfersAfterForceRenounce() N/A N/A 🟢 -44
Test test_GetStorage_ReturnsCorrectOwner() N/A N/A 🟢 -44
Test test_StorageCollision_DocumentedBug() N/A N/A 🔴 +44
Test test_PendingOwner_ReturnsCurrentPendingOwner() N/A N/A 🔴 +42
Test test_CancelPendingTransfer() N/A N/A 🔴 +36
Test test_AcceptOwnership_CurrentOwnerCanCall() N/A N/A 🟢 -35
Test test_TransferOwnership_EmitsOwnershipTransferStartedEvent() N/A N/A 🔴 +23
Test test_RenounceOwnership_CannotBeCompleted() N/A N/A 🔴 +23
Test test_StorageSlot_CurrentlyCollides() N/A N/A 🔴 +23
Test test_TransferOwnership_CanUpdatePendingOwner() N/A N/A 🟢 -22
Test test_GetStorage_ReturnsCorrectPendingOwner() N/A N/A 🟢 -22
Test test_TransferOwnership_LibraryDoesNotCheckCaller() N/A N/A 🔴 +22
Test test_StorageSlot_UsesCorrectPosition() N/A N/A 🟢 -22
Test testFuzz_TransferOwnership_AnyCaller() N/A N/A 🔴 +22
Test test_AcceptOwnership_NoPendingOwner_SetsOwnerToZero() N/A N/A 🟢 -22
Test test_RevertWhen_TransferOwnership_FromRenouncedOwner() N/A N/A 🔴 +22
Test test_Owner_ReturnsCurrentOwner() N/A N/A 🔴 +22
Test test_Owner_ReturnsCorrectInitialOwner() N/A N/A 🟢 -22
Test testFuzz_SequentialTransfers() N/A N/A 🔴 +19
Test test_SequentialTransfers() N/A N/A 🔴 +18
Test test_AcceptOwnership_LibraryAllowsAnyCaller() N/A N/A 🟢 -18
Test testFuzz_AcceptOwnership_AnyCaller() N/A N/A 🔴 +18
Test test_AcceptOwnership_ClearsPendingOwner() N/A N/A 🔴 +18
Test testFuzz_AcceptOwnership() N/A N/A 🔴 +18
Test test_AcceptOwnership_EmitsOwnershipTransferredEvent() N/A N/A 🔴 +18
Test test_TransferOwnership_MultipleTransfers() N/A N/A 🔴 +18
Test test_RenounceOwnership_PreventsAllFurtherTransfers() N/A N/A 🔴 +18
Test test_MultiplePendingChanges_OnlyLastOneMatters() N/A N/A 🟢 -17
Test test_AcceptOwnership_CompletesTransfer() N/A N/A 🔴 +17
Test test_TransferToSelf() N/A N/A 🟢 -17
Test test_TransferOwnership_EmitsOwnershipTransferredEvent() N/A N/A 🟢 -16
Test testFuzz_TransferOwnership() N/A N/A 🟢 -16
Test test_RenounceOwnership_EmitsEvent() N/A N/A 🟢 -16
Test testFuzz_RevertWhen_UnauthorizedCaller() N/A N/A 🟢 -16
Test test_RevertWhen_TransferOwnership_CalledByPreviousOwner() N/A N/A 🟢 -10
Test test_StorageSlot_Consistency() N/A N/A 🔴 +7
Test test_TransferOwnership_ImmediateTransfer() N/A N/A 🔴 +6
Test test_Owner_ReturnsZeroWhenRenounced() N/A N/A 🔴 +6
ℹ️ About this report

This report compares gas usage between the base branch and this PR using forge snapshot.

  • 🟢 indicates a gas improvement (reduction)
  • 🔴 indicates a gas regression (increase)
  • Functions not shown have unchanged gas costs

To run this locally:

# Generate snapshot for current branch
forge snapshot

# Compare with another branch
git checkout main
forge snapshot --diff .gas-snapshot

Last updated: Sun, 23 Nov 2025 14:25:52 GMT for commit 8db0d28

@maxnorm maxnorm changed the title Add missing test OwnerFacet & LibOwnerTwoStep Add missing test for OwnerFacet & LibOwnerTwoStep Nov 23, 2025
@maxnorm maxnorm merged commit 8ce1ea3 into Perfect-Abstractions:main Nov 23, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant