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.

Sign up for GitHub

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

Added tests #975

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
+887 −27
Open

Added tests #975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
63fc374
Added tests
mirooon Feb 4, 2025
8e283f7
Merge branch 'main' into added-tests
mirooon Feb 5, 2025
6c7c5b5
Added blank lines, changed tests naming
mirooon Feb 11, 2025
3046420
updated AccessManagerFacet tests
mirooon Feb 12, 2025
4616bbd
Updated DexManagerFacet tests
mirooon Feb 12, 2025
a0e3aaa
Updated CBridge and CBridgeFacetPacked tests
mirooon Feb 12, 2025
770e34e
adjustments
mirooon Feb 12, 2025
89dc4a8
Updated CBridge and CBridgeFacetPacket tests
mirooon Feb 13, 2025
a3301af
Merge branch 'main' into added-tests
mirooon Feb 13, 2025
b406646
adjustments
mirooon Feb 13, 2025
e75b9c9
adjustments
mirooon Feb 13, 2025
406a20a
adjustmnets
mirooon Feb 13, 2025
694d999
adjustments
mirooon Feb 13, 2025
c9c5f9d
changed names
mirooon Feb 13, 2025
983b503
updates
mirooon Feb 13, 2025
c5e6f5b
updates
mirooon Feb 13, 2025
9a5c427
adjustments
mirooon Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 151 additions & 10 deletions test/solidity/Facets/AccessManagerFacet.t.sol
View file
Open in desktop
mirooon marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.17;

import { AccessManagerFacet } from "lifi/Facets/AccessManagerFacet.sol";
import { UnAuthorized } from "lifi/Errors/GenericErrors.sol";
import { UnAuthorized, CannotAuthoriseSelf, OnlyContractOwner } from "lifi/Errors/GenericErrors.sol";
import { TestBase, LibAccess, console, LiFiDiamond } from "../utils/TestBase.sol";

contract RestrictedContract {
Expand All @@ -22,12 +22,16 @@ contract AccessManagerFacetTest is TestBase {
accessMgr = new AccessManagerFacet();
restricted = new RestrictedContract();

bytes4[] memory functionSelectors = new bytes4[](1);
functionSelectors[0] = accessMgr.setCanExecute.selector;
addFacet(diamond, address(accessMgr), functionSelectors);
bytes4[] memory allowedFunctionSelectors = new bytes4[](2);
allowedFunctionSelectors[0] = accessMgr.setCanExecute.selector;
allowedFunctionSelectors[1] = accessMgr
.addressCanExecuteMethod
.selector;
addFacet(diamond, address(accessMgr), allowedFunctionSelectors);

functionSelectors[0] = restricted.restrictedMethod.selector;
addFacet(diamond, address(restricted), functionSelectors);
bytes4[] memory restrictedFunctionSelectors = new bytes4[](1);
restrictedFunctionSelectors[0] = restricted.restrictedMethod.selector;
addFacet(diamond, address(restricted), restrictedFunctionSelectors);

accessMgr = AccessManagerFacet(address(diamond));
restricted = RestrictedContract(address(diamond));
Expand All @@ -36,23 +40,33 @@ contract AccessManagerFacetTest is TestBase {
setFacetAddressInTestBase(address(accessMgr), "AccessManagerFacet");
}

function testAccessIsRestricted() public {
function test_AccessIsRestrictedFromNotOwner() public {
vm.startPrank(USER_SENDER);

vm.expectRevert(UnAuthorized.selector);
vm.prank(address(0xb33f));

restricted.restrictedMethod();
vm.stopPrank();
}

function testCanGrantAccess() public {
function test_CanGrantAccessByOwner() public {
vm.startPrank(USER_DIAMOND_OWNER);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
true
);

vm.stopPrank();

vm.prank(address(0xb33f));
restricted.restrictedMethod();
}

function testCanRemoveAccess() public {
function test_CanRemoveAccessByOwner() public {
vm.startPrank(USER_DIAMOND_OWNER);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
Expand All @@ -63,8 +77,135 @@ contract AccessManagerFacetTest is TestBase {
address(0xb33f),
false
);

vm.stopPrank();

vm.expectRevert(UnAuthorized.selector);

vm.prank(address(0xb33f));
restricted.restrictedMethod();
}

function testRevert_CannotSetMethodAccessForAccessManager() public {
vm.startPrank(USER_DIAMOND_OWNER);

vm.expectRevert(CannotAuthoriseSelf.selector);

accessMgr.setCanExecute(
mirooon marked this conversation as resolved.
Show resolved Hide resolved
AccessManagerFacet.setCanExecute.selector,
address(accessMgr),
true
);

vm.stopPrank();
}
Comment on lines +89 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify the test coverage for self-authorization.

The test correctly verifies that the access manager cannot authorize itself, but it should also verify that other contracts can still be authorized.

🏁 Script executed:

#!/bin/bash
# Description: Check if there are other tests that verify successful authorization of other contracts

rg -A 5 "test.*CanSetMethodAccess.*" || echo "No additional tests found for successful authorization"

Length of output: 157

Missing Test for Positive Authorization
The current test (testRevert_CannotSetMethodAccessForAccessManager) correctly checks that the access manager cannot authorize itself. However, our verification indicates that there are no tests confirming that other contracts can be successfully authorized. Please add a test case to validate that a contract other than the access manager can be granted execution rights using the setCanExecute method.

  • Add a positive test in test/solidity/Facets/AccessManagerFacet.t.sol (or another appropriate test file) that verifies successful authorization of a different contract.


function testRevert_FailsIfNonOwnerTriesToGrantAccess() public {
vm.startPrank(USER_SENDER);

vm.expectRevert(OnlyContractOwner.selector);

accessMgr.setCanExecute(
AccessManagerFacet.setCanExecute.selector,
address(0xb33f),
true
);

vm.stopPrank();
}

function test_DefaultAccessIsFalse() public {
bool canExecute = accessMgr.addressCanExecuteMethod(
RestrictedContract.restrictedMethod.selector,
address(0xb33f)
);

assertEq(canExecute, false, "Default access should be false");
}

function test_CanCheckGrantedAccess() public {
vm.startPrank(USER_DIAMOND_OWNER);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
true
);

vm.stopPrank();

bool canExecute = accessMgr.addressCanExecuteMethod(
RestrictedContract.restrictedMethod.selector,
address(0xb33f)
);

assertEq(canExecute, true, "Access should be granted");
}

function test_CanCheckRevokedAccess() public {
vm.startPrank(USER_DIAMOND_OWNER);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
true
);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
false
);

vm.stopPrank();

bool canExecute = accessMgr.addressCanExecuteMethod(
RestrictedContract.restrictedMethod.selector,
address(0xb33f)
);

assertEq(canExecute, false, "Access should be revoked");
}

function test_DifferentMethodSelectorReturnsFalse() public {
vm.startPrank(USER_DIAMOND_OWNER);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
true
);

vm.stopPrank();

bool canExecute = accessMgr.addressCanExecuteMethod(
bytes4(keccak256("anotherMethod()")),
address(0xb33f)
);

assertEq(
canExecute,
false,
"Different method selector should return false"
);
}

function test_DifferentExecutorReturnsFalse() public {
vm.startPrank(USER_DIAMOND_OWNER);

accessMgr.setCanExecute(
RestrictedContract.restrictedMethod.selector,
address(0xb33f),
true
);

vm.stopPrank();

bool canExecute = accessMgr.addressCanExecuteMethod(
RestrictedContract.restrictedMethod.selector,
address(0xcafe)
);

assertEq(canExecute, false, "Different executor should return false");
}
}
19 changes: 19 additions & 0 deletions test/solidity/Facets/AcrossFacetPacked.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IAcrossSpokePool } from "lifi/Interfaces/IAcrossSpokePool.sol";
import { LibAsset, IERC20 } from "lifi/Libraries/LibAsset.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { TestBase } from "../utils/TestBase.sol";
import { MockFailingContract } from "../utils/MockFailingContract.sol";
import { ERC20 } from "solmate/tokens/ERC20.sol";
import { LiFiDiamond } from "../utils/DiamondTest.sol";
import { console2 } from "forge-std/console2.sol";
Expand Down Expand Up @@ -616,4 +617,22 @@ contract AcrossFacetPackedTest is TestBase {
);
vm.stopPrank();
}

function testRevert_FailIfCallToExternalContractFails() public {
vm.startPrank(USER_DIAMOND_OWNER);

MockFailingContract failingContract = new MockFailingContract();

vm.expectRevert(AcrossFacetPacked.WithdrawFailed.selector);

acrossStandAlone.executeCallAndWithdraw(
address(failingContract),
WITHDRAW_REWARDS_CALLDATA,
ADDRESS_USDT,
address(this),
amountUSDT
);

vm.startPrank(USER_DIAMOND_OWNER);
}
}
21 changes: 21 additions & 0 deletions test/solidity/Facets/AcrossFacetV3.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { AcrossFacetV3 } from "lifi/Facets/AcrossFacetV3.sol";
import { IAcrossSpokePool } from "lifi/Interfaces/IAcrossSpokePool.sol";
import { LibUtil } from "lifi/Libraries/LibUtil.sol";
import { LibSwap } from "lifi/Libraries/LibSwap.sol";
import { InformationMismatch } from "src/Errors/GenericErrors.sol";

// Stub AcrossFacetV3 Contract
contract TestAcrossFacetV3 is AcrossFacetV3 {
Expand Down Expand Up @@ -271,4 +272,24 @@ contract AcrossFacetV3Test is TestBaseFacet {
true
);
}

function testRevert_RevertIfReceiverAddressesDontMatch() public {
vm.startPrank(USER_SENDER);

usdc.approve(address(acrossFacetV3), type(uint256).max);

bridgeData.hasDestinationCall = false;

bridgeData.receiver = USER_RECEIVER;
validAcrossData.receiverAddress = address(0xbadbeef);

vm.expectRevert(InformationMismatch.selector);

acrossFacetV3.startBridgeTokensViaAcrossV3(
bridgeData,
validAcrossData
);

vm.stopPrank();
}
}
67 changes: 65 additions & 2 deletions test/solidity/Facets/AmarokFacetPacked.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ contract AmarokFacetPackedTest is TestBase {
amarokFacetPacked = new AmarokFacetPacked(amarok, address(this));
amarokStandAlone = new AmarokFacetPacked(amarok, address(this));

bytes4[] memory functionSelectors = new bytes4[](9);
bytes4[] memory functionSelectors = new bytes4[](10);
functionSelectors[0] = amarokFacetPacked.setApprovalForBridge.selector;
functionSelectors[1] = amarokFacetPacked
.startBridgeTokensViaAmarokERC20PackedPayFeeWithAsset
Expand All @@ -73,6 +73,7 @@ contract AmarokFacetPackedTest is TestBase {
functionSelectors[8] = amarokFacetPacked
.decode_startBridgeTokensViaAmarokERC20PackedPayFeeWithNative
.selector;
functionSelectors[9] = amarokFacetPacked.getChainIdForDomain.selector;

// add facet to diamond
addFacet(diamond, address(amarokFacetPacked), functionSelectors);
Expand Down Expand Up @@ -412,7 +413,69 @@ contract AmarokFacetPackedTest is TestBase {
assertEq(amarokData.relayerFee == defaultRelayerFee, true);
}

function test_revert_cannotUseRelayerFeeAboveUint128Max_ERC20() public {
struct DomainChainTestCase {
uint32 domainId;
uint32 expectedChainId;
string description;
}

function test_CanGetChainIdForValidDomains() public {
DomainChainTestCase[] memory testCases = new DomainChainTestCase[](8);
testCases[0] = DomainChainTestCase({
domainId: 6648936,
expectedChainId: 1,
description: "ETH domainId should return chainId 1"
});
testCases[1] = DomainChainTestCase({
domainId: 1886350457,
expectedChainId: 137,
description: "POL domainId should return chainId 137"
});
testCases[2] = DomainChainTestCase({
domainId: 6450786,
expectedChainId: 56,
description: "BSC domainId should return chainId 56"
});
testCases[3] = DomainChainTestCase({
domainId: 1869640809,
expectedChainId: 10,
description: "OPT domainId should return chainId 10"
});
testCases[4] = DomainChainTestCase({
domainId: 6778479,
expectedChainId: 100,
description: "GNO domainId should return chainId 100"
});
testCases[5] = DomainChainTestCase({
domainId: 1634886255,
expectedChainId: 42161,
description: "ARB domainId should return chainId 42161"
});
testCases[6] = DomainChainTestCase({
domainId: 1818848877,
expectedChainId: 59144,
description: "LIN domainId should return chainId 59144"
});
testCases[7] = DomainChainTestCase({
domainId: 9999999,
expectedChainId: 0,
description: "Unknown domainId should return 0"
});

for (uint256 i = 0; i < testCases.length; i++) {
uint32 result = amarokFacetPacked.getChainIdForDomain(
testCases[i].domainId
);

assertEq(
result,
testCases[i].expectedChainId,
testCases[i].description
);
}
}

function testRevert_cannotUseRelayerFeeAboveUint128Max_ERC20() public {
uint256 invalidRelayerFee = uint256(type(uint128).max) + 1;

vm.expectRevert("relayerFee value passed too big to fit in uint128");
Expand Down
Loading
Loading