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

[NO-3155] Fix usage of pranks in functions to make suite work with foundry@nightly #677

Merged
merged 2 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly-d4f626bb7f96d46358997d4b27f79358cb2b3401
version: nightly

- name: Install Hardhat Dependencies
run: yarn install --frozen-lockfile --prefer-offline --ignore-scripts
Expand Down
63 changes: 18 additions & 45 deletions test/Market.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ contract Market_swap_revertsWhenUnsafeERC20TransferFails is UpgradeableMarket {
}

function test() external {
vm.startPrank(owner);
vm.prank(owner);
amiecorso marked this conversation as resolved.
Show resolved Hide resolved
vm.expectRevert(ERC20TransferFailed.selector);
_market.swap(
owner,
Expand All @@ -108,7 +108,6 @@ contract Market_swap_revertsWhenUnsafeERC20TransferFails is UpgradeableMarket {
signedPermit.r,
signedPermit.s
);
vm.stopPrank();
}
}

Expand Down Expand Up @@ -216,7 +215,6 @@ contract Market_replace is MarketReplaceTestHelper {
function setUp() external {
_listRemovals();
_createCertificate();

_market.grantRole({
role: _market.MARKET_ADMIN_ROLE(),
account: _namedAccounts.admin
Expand All @@ -227,13 +225,14 @@ contract Market_replace is MarketReplaceTestHelper {
});
uint256 amount = _market.calculateCheckoutTotal(_amountToReplace);
vm.startPrank(_namedAccounts.admin);
_removal.release(_removalIds[0], _originalCertificateAmount);
assertEq(_certificate.getNrtDeficit(), _originalCertificateAmount);
_bpNori.deposit(_namedAccounts.admin, abi.encode(amount));
_bpNori.approve(address(_market), amount);
_removal.release(_removalIds[0], _originalCertificateAmount);
vm.stopPrank();
amiecorso marked this conversation as resolved.
Show resolved Hide resolved
}

function test() external {
assertEq(_certificate.getNrtDeficit(), _originalCertificateAmount);
amiecorso marked this conversation as resolved.
Show resolved Hide resolved
vm.expectEmit(true, true, false, true);
emit UpdateCertificate(
_certificateTokenId,
Expand All @@ -244,6 +243,7 @@ contract Market_replace is MarketReplaceTestHelper {
address(_bpNori),
_market.getPriceMultiple()
);
vm.prank(_namedAccounts.admin);
_market.replace({
treasury: _namedAccounts.admin,
certificateId: _certificateTokenId,
Expand All @@ -255,7 +255,6 @@ contract Market_replace is MarketReplaceTestHelper {
_certificate.getNrtDeficit(),
_originalCertificateAmount - _amountToReplace
);
vm.stopPrank();
}
}

Expand All @@ -265,37 +264,22 @@ contract Market_replace_reverts_ReplacementAmountExceedsNrtDeficit is
function setUp() external {
_listRemovals();
_createCertificate();

_market.grantRole({
role: _market.MARKET_ADMIN_ROLE(),
account: _namedAccounts.admin
});
_removal.grantRole({
role: _removal.RELEASER_ROLE(),
account: _namedAccounts.admin
});
uint256 amount = _market.calculateCheckoutTotal(_amountToReplace);
vm.startPrank(_namedAccounts.admin);
assertEq(_certificate.getNrtDeficit(), 0); // no discrepancy
_bpNori.deposit(_namedAccounts.admin, abi.encode(amount));
_bpNori.approve(address(_market), amount);
amiecorso marked this conversation as resolved.
Show resolved Hide resolved
}

function test() external {
// using low level call to get around limitation of vm.expectRevert
amiecorso marked this conversation as resolved.
Show resolved Hide resolved
// see https://book.getfoundry.sh/cheatcodes/expect-revert
(bool success, ) = address(_market).call(
abi.encodeWithSignature(
"replace(address,uint256,uint256,uint256[],uint256[])",
_namedAccounts.admin,
_certificateTokenId,
_amountToReplace,
new uint256[](1).fill(_removalIds[0]),
new uint256[](1).fill(_amountToReplace)
)
vm.prank(_namedAccounts.admin);
vm.expectRevert(ReplacementAmountExceedsNrtDeficit.selector);
_market.replace(
_namedAccounts.admin,
_certificateTokenId,
_amountToReplace,
new uint256[](1).fill(_removalIds[0]),
new uint256[](1).fill(_amountToReplace)
);
assertFalse(success, "expectRevert: call did not revert");
vm.stopPrank();
}
}

Expand All @@ -309,14 +293,10 @@ contract Market_replace_reverts_CertificateNotYetMinted is
role: _market.MARKET_ADMIN_ROLE(),
account: _namedAccounts.admin
});
uint256 amount = _market.calculateCheckoutTotal(_amountToReplace);
vm.startPrank(_namedAccounts.admin);

_bpNori.deposit(_namedAccounts.admin, abi.encode(amount));
_bpNori.approve(address(_market), amount);
}

function test() external {
vm.prank(_namedAccounts.admin);
vm.expectRevert(
abi.encodeWithSelector(
CertificateNotYetMinted.selector,
Expand All @@ -330,7 +310,6 @@ contract Market_replace_reverts_CertificateNotYetMinted is
removalIdsBeingReplaced: new uint256[](1).fill(_removalIds[0]),
amountsBeingReplaced: new uint256[](1).fill(_amountToReplace)
});
vm.stopPrank();
}
}

Expand All @@ -348,24 +327,20 @@ contract Market_replace_reverts_ReplacementAmountMismatch is
role: _removal.RELEASER_ROLE(),
account: _namedAccounts.admin
});
uint256 amount = _market.calculateCheckoutTotal(_amountToReplace);
vm.startPrank(_namedAccounts.admin);
vm.prank(_namedAccounts.admin);
_removal.release(_removalIds[0], _originalCertificateAmount);
assertEq(_certificate.getNrtDeficit(), _originalCertificateAmount);
_bpNori.deposit(_namedAccounts.admin, abi.encode(amount));
_bpNori.approve(address(_market), amount);
}

function test() external {
vm.expectRevert(ReplacementAmountMismatch.selector);
vm.prank(_namedAccounts.admin);
_market.replace({
treasury: _namedAccounts.admin,
certificateId: _certificateTokenId,
totalAmountToReplace: _amountToReplace / 2, // mismatch with amountsBeingReplaced
removalIdsBeingReplaced: new uint256[](1).fill(_removalIds[0]),
amountsBeingReplaced: new uint256[](1).fill(_amountToReplace)
});
vm.stopPrank();
}
}

Expand Down Expand Up @@ -509,7 +484,7 @@ contract Market_swap_emits_and_skips_transfer_when_transferring_wrong_erc20_to_r
}

function test() external {
vm.startPrank(owner);
vm.prank(owner);
vm.recordLogs();
_market.swap(
owner,
Expand All @@ -520,7 +495,6 @@ contract Market_swap_emits_and_skips_transfer_when_transferring_wrong_erc20_to_r
signedPermit.r,
signedPermit.s
);
vm.stopPrank();
Vm.Log[] memory entries = vm.getRecordedLogs();
bool containsTransferSkippedEventSelector = false;
for (uint256 i = 0; i < entries.length; ++i) {
Expand Down Expand Up @@ -590,10 +564,9 @@ contract Market_swapWithoutFee_emits_and_skips_transfer_when_transferring_wrong_
}

function test() external {
vm.startPrank(owner);
vm.prank(owner);
vm.recordLogs();
_market.swapWithoutFee(owner, owner, certificateAmount);
vm.stopPrank();
Vm.Log[] memory entries = vm.getRecordedLogs();
bool containsTransferSkippedEventSelector = false;
for (uint256 i = 0; i < entries.length; ++i) {
Expand Down
3 changes: 1 addition & 2 deletions test/MarketGasExplorer.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ contract MarketGasExplorer is UpgradeableMarket, QuickSort {
_bpNori
);

vm.startPrank(_owner);
vm.prank(_owner);
uint256 gasLeft1 = gasleft();
_market.swap(
_owner,
Expand All @@ -256,7 +256,6 @@ contract MarketGasExplorer is UpgradeableMarket, QuickSort {
);
uint256 gasLeft2 = gasleft();
uint256 gasDelta = gasLeft1 - gasLeft2 - 100; // https://ethereum.stackexchange.com/questions/132323/transaction-gas-cost-in-foundry-forge-unit-tests
vm.stopPrank();
_gasAmounts.push(gasDelta);
}

Expand Down
6 changes: 3 additions & 3 deletions test/Removal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ contract Removal_migrate_revertsIfRemovalBalanceSumDifferentFromCertificateAmoun
++index;
}
}
vm.startPrank(_namedAccounts.admin);
}

function test() external {
vm.prank(_namedAccounts.admin);
vm.expectRevert("Incorrect supply allocation");
_removal.migrate({
ids: idsForAllSuppliers,
Expand Down Expand Up @@ -184,7 +184,6 @@ contract Removal_migrate is UpgradeableMarket {
++index;
}
}
vm.startPrank(_namedAccounts.admin);
}

event Migrate(
Expand All @@ -196,6 +195,7 @@ contract Removal_migrate is UpgradeableMarket {
);

function test() external {
vm.prank(_namedAccounts.admin);
vm.recordLogs();
_removal.migrate({
ids: idsForAllSuppliers,
Expand Down Expand Up @@ -281,10 +281,10 @@ contract Removal_migrate_gasLimit is UpgradeableMarket {
++index;
}
}
vm.startPrank(_namedAccounts.admin);
}

function test() external {
vm.prank(_namedAccounts.admin);
uint256 initialGas = gasleft();
_removal.migrate({
ids: idsForAllSuppliers,
Expand Down
4 changes: 2 additions & 2 deletions test/RestrictedNORI.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ contract RestrictedNORI_transfers_revert is UpgradeableMarket {

function testSafeTransferFromReverts() external {
address newSupplier = address(uint160(100));
vm.startPrank(_namedAccounts.supplier);
vm.prank(_namedAccounts.supplier);
vm.expectRevert(FunctionDisabled.selector);
_rNori.safeTransferFrom(
_namedAccounts.supplier,
Expand All @@ -213,7 +213,7 @@ contract RestrictedNORI_transfers_revert is UpgradeableMarket {

function testSafeBatchTransferFromReverts() external {
address newSupplier = address(uint160(100));
vm.startPrank(_namedAccounts.supplier);
vm.prank(_namedAccounts.supplier);
vm.expectRevert(FunctionDisabled.selector);
_rNori.safeBatchTransferFrom(
_namedAccounts.supplier,
Expand Down
13 changes: 4 additions & 9 deletions test/checkout.int.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ contract Checkout_buyingWithAlternateERC20 is Checkout {
_erc20
);
vm.recordLogs();
vm.startPrank(owner);
vm.prank(owner);
_market.swap(
owner,
owner,
Expand All @@ -799,7 +799,6 @@ contract Checkout_buyingWithAlternateERC20 is Checkout {
signedPermit.r,
signedPermit.s
);
vm.stopPrank();

Vm.Log[] memory entries = vm.getRecordedLogs();
bool containsCreateCertificateEventSelector = false;
Expand Down Expand Up @@ -901,7 +900,7 @@ contract Checkout_buyingWithAlternateERC20_floatingPointPriceMultiple is
_erc20
);
vm.recordLogs();
vm.startPrank(owner);
vm.prank(owner);
_market.swap(
owner,
owner,
Expand All @@ -911,7 +910,6 @@ contract Checkout_buyingWithAlternateERC20_floatingPointPriceMultiple is
signedPermit.r,
signedPermit.s
);
vm.stopPrank();
Vm.Log[] memory entries = vm.getRecordedLogs();
bool containsCreateCertificateEventSelector = false;
for (uint256 i = 0; i < entries.length; ++i) {
Expand Down Expand Up @@ -985,15 +983,14 @@ contract Checkout_buyingWithCustomFee is Checkout {
}

function test() external {
vm.startPrank(owner);
vm.prank(owner);
vm.recordLogs();
_market.swapWithoutFeeSpecialOrder(
owner,
owner,
certificateAmount,
customFee
);
vm.stopPrank();

Vm.Log[] memory entries = vm.getRecordedLogs();
uint256 createCertificateEventIndex = 6;
Expand Down Expand Up @@ -1058,7 +1055,7 @@ contract Checkout_buyingFromSingleSupplierWithCustomFee is Checkout {
}

function test() external {
vm.startPrank(owner);
vm.prank(owner);
vm.recordLogs();
_market.swapFromSupplierWithoutFeeSpecialOrder(
owner,
Expand All @@ -1067,8 +1064,6 @@ contract Checkout_buyingFromSingleSupplierWithCustomFee is Checkout {
_namedAccounts.supplier,
customFee
);
vm.stopPrank();

Vm.Log[] memory entries = vm.getRecordedLogs();
uint256 createCertificateEventIndex = 6;
assertEq(
Expand Down