Skip to content

Commit

Permalink
refactor: capitalize immutable variables (#700)
Browse files Browse the repository at this point in the history 
* refactor: capitalize immutable variables in NoDelegateCall

build: remove "immutable-vars-naming" from solhint file

* test: make asset and holder immutable in fork tests

test: capitalize constans in fork tests
  • Loading branch information
@andreivladbrg
andreivladbrg authored Oct 2, 2023
1 parent 924df97 commit 8ca98d3
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 73 deletions.
1 change: 0 additions & 1 deletion .solhint.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"contract-name-camelcase": "off",
"const-name-snakecase": "off",
"custom-errors": "off",
"immutable-vars-naming": "off",
"func-name-mixedcase": "off",
"func-visibility": ["error", { "ignoreConstructors": true }],
"max-line-length": ["error", 123],
Expand Down
8 changes: 4 additions & 4 deletions src/abstracts/NoDelegateCall.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { Errors } from "../libraries/Errors.sol";
/// @notice This contract implements logic to prevent delegate calls.
abstract contract NoDelegateCall {
/// @dev The address of the original contract that was deployed.
address private immutable _original;
address private immutable ORIGINAL;

/// @dev Sets the original contract address.
constructor() {
_original = address(this);
ORIGINAL = address(this);
}

/// @notice Prevents delegate calls.
Expand All @@ -23,11 +23,11 @@ abstract contract NoDelegateCall {
/// @dev This function checks whether the current call is a delegate call, and reverts if it is.
///
/// - A private function is used instead of inlining this logic in a modifier because Solidity copies modifiers into
/// every function that uses them. The `_original` address would get copied in every place the modifier is used,
/// every function that uses them. The `ORIGINAL` address would get copied in every place the modifier is used,
/// which would increase the contract size. By using a function instead, we can avoid this duplication of code
/// and reduce the overall size of the contract.
function _preventDelegateCall() private view {
if (address(this) != _original) {
if (address(this) != ORIGINAL) {
revert Errors.DelegateCall();
}
}
Expand Down
32 changes: 16 additions & 16 deletions test/fork/Fork.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ abstract contract Fork_Test is Base_Test {
CONSTANTS
//////////////////////////////////////////////////////////////////////////*/

IERC20 internal immutable asset;
address internal immutable holder;
IERC20 internal immutable ASSET;
address internal immutable HOLDER;

/*//////////////////////////////////////////////////////////////////////////
VARIABLES
Expand All @@ -25,9 +25,9 @@ abstract contract Fork_Test is Base_Test {
CONSTRUCTOR
//////////////////////////////////////////////////////////////////////////*/

constructor(IERC20 asset_, address holder_) {
asset = asset_;
holder = holder_;
constructor(IERC20 asset, address holder) {
ASSET = asset;
HOLDER = holder;
}

/*//////////////////////////////////////////////////////////////////////////
Expand All @@ -47,11 +47,11 @@ abstract contract Fork_Test is Base_Test {
// Label the contracts.
labelContracts();

// Make the asset holder the caller in this test suite.
vm.startPrank({ msgSender: holder });
// Make the ASSET HOLDER the caller in this test suite.
vm.startPrank({ msgSender: HOLDER });

// Query the initial balance of the asset holder.
initialHolderBalance = asset.balanceOf(holder);
// Query the initial balance of the ASSET HOLDER.
initialHolderBalance = ASSET.balanceOf(HOLDER);
}

/*//////////////////////////////////////////////////////////////////////////
Expand All @@ -63,20 +63,20 @@ abstract contract Fork_Test is Base_Test {
// The protocol does not allow the zero address to interact with it.
vm.assume(sender != address(0) && recipient != address(0) && broker != address(0));

// The goal is to not have overlapping users because the asset balance tests would fail otherwise.
// The goal is to not have overlapping users because the ASSET balance tests would fail otherwise.
vm.assume(sender != recipient && sender != broker && recipient != broker);
vm.assume(sender != holder && recipient != holder && broker != holder);
vm.assume(sender != HOLDER && recipient != HOLDER && broker != HOLDER);
vm.assume(sender != sablierContract && recipient != sablierContract && broker != sablierContract);

// Avoid users blacklisted by USDC or USDT.
assumeNoBlacklisted(address(asset), sender);
assumeNoBlacklisted(address(asset), recipient);
assumeNoBlacklisted(address(asset), broker);
assumeNoBlacklisted(address(ASSET), sender);
assumeNoBlacklisted(address(ASSET), recipient);
assumeNoBlacklisted(address(ASSET), broker);
}

/// @dev Labels the most relevant contracts.
function labelContracts() internal {
vm.label({ account: address(asset), newLabel: IERC20Metadata(address(asset)).symbol() });
vm.label({ account: holder, newLabel: "Holder" });
vm.label({ account: address(ASSET), newLabel: IERC20Metadata(address(ASSET)).symbol() });
vm.label({ account: HOLDER, newLabel: "HOLDER" });
}
}
32 changes: 16 additions & 16 deletions test/fork/LockupDynamic.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
CONSTRUCTOR
//////////////////////////////////////////////////////////////////////////*/

constructor(IERC20 asset_, address holder_) Fork_Test(asset_, holder_) { }
constructor(IERC20 asset, address holder) Fork_Test(asset, holder) { }

/*//////////////////////////////////////////////////////////////////////////
SET-UP FUNCTION
Expand All @@ -25,7 +25,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {

// Approve {SablierV2LockupDynamic} to transfer the holder's assets.
// We use a low-level call to ignore reverts because the asset can have the missing return value bug.
(bool success,) = address(asset).call(abi.encodeCall(IERC20.approve, (address(lockupDynamic), MAX_UINT256)));
(bool success,) = address(ASSET).call(abi.encodeCall(IERC20.approve, (address(lockupDynamic), MAX_UINT256)));
success;
}

Expand Down Expand Up @@ -136,21 +136,21 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {

// Set the fuzzed protocol fee.
changePrank({ msgSender: users.admin });
comptroller.setProtocolFee({ asset: asset, newProtocolFee: params.protocolFee });
comptroller.setProtocolFee({ asset: ASSET, newProtocolFee: params.protocolFee });

// Make the holder the caller.
changePrank(holder);
changePrank(HOLDER);

/*//////////////////////////////////////////////////////////////////////////
CREATE
//////////////////////////////////////////////////////////////////////////*/

// Load the pre-create protocol revenues.
vars.initialProtocolRevenues = lockupDynamic.protocolRevenues(asset);
vars.initialProtocolRevenues = lockupDynamic.protocolRevenues(ASSET);

// Load the pre-create asset balances.
vars.balances =
getTokenBalances(address(asset), Solarray.addresses(address(lockupDynamic), params.broker.account));
getTokenBalances(address(ASSET), Solarray.addresses(address(lockupDynamic), params.broker.account));
vars.initialLockupDynamicBalance = vars.balances[0];
vars.initialBrokerBalance = vars.balances[1];

Expand All @@ -164,11 +164,11 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
vm.expectEmit({ emitter: address(lockupDynamic) });
emit CreateLockupDynamicStream({
streamId: vars.streamId,
funder: holder,
funder: HOLDER,
sender: params.sender,
recipient: params.recipient,
amounts: vars.createAmounts,
asset: asset,
asset: ASSET,
cancelable: true,
transferable: params.transferable,
segments: params.segments,
Expand All @@ -179,7 +179,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
// Create the stream.
lockupDynamic.createWithMilestones(
LockupDynamic.CreateWithMilestones({
asset: asset,
asset: ASSET,
broker: params.broker,
cancelable: true,
transferable: params.transferable,
Expand All @@ -199,7 +199,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
// Assert that the stream has been created.
LockupDynamic.Stream memory actualStream = lockupDynamic.getStream(vars.streamId);
assertEq(actualStream.amounts, Lockup.Amounts(vars.createAmounts.deposit, 0, 0));
assertEq(actualStream.asset, asset, "asset");
assertEq(actualStream.asset, ASSET, "asset");
assertEq(actualStream.endTime, vars.range.end, "endTime");
assertEq(actualStream.isCancelable, vars.isCancelable, "isCancelable");
assertEq(actualStream.isDepleted, false, "isDepleted");
Expand Down Expand Up @@ -227,7 +227,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
assertEq(vars.actualNextStreamId, vars.expectedNextStreamId, "post-create nextStreamId");

// Assert that the protocol fee has been recorded.
vars.actualProtocolRevenues = lockupDynamic.protocolRevenues(asset);
vars.actualProtocolRevenues = lockupDynamic.protocolRevenues(ASSET);
vars.expectedProtocolRevenues = vars.initialProtocolRevenues + vars.createAmounts.protocolFee;
assertEq(vars.actualProtocolRevenues, vars.expectedProtocolRevenues, "post-create protocolRevenues");

Expand All @@ -238,7 +238,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {

// Load the post-create asset balances.
vars.balances =
getTokenBalances(address(asset), Solarray.addresses(address(lockupDynamic), holder, params.broker.account));
getTokenBalances(address(ASSET), Solarray.addresses(address(lockupDynamic), HOLDER, params.broker.account));
vars.actualLockupDynamicBalance = vars.balances[0];
vars.actualHolderBalance = vars.balances[1];
vars.actualBrokerBalance = vars.balances[2];
Expand Down Expand Up @@ -281,7 +281,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
if (params.withdrawAmount > 0) {
// Load the pre-withdraw asset balances.
vars.initialLockupDynamicBalance = vars.actualLockupDynamicBalance;
vars.initialRecipientBalance = asset.balanceOf(params.recipient);
vars.initialRecipientBalance = ASSET.balanceOf(params.recipient);

// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupDynamic) });
Expand Down Expand Up @@ -315,7 +315,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {

// Load the post-withdraw asset balances.
vars.balances =
getTokenBalances(address(asset), Solarray.addresses(address(lockupDynamic), params.recipient));
getTokenBalances(address(ASSET), Solarray.addresses(address(lockupDynamic), params.recipient));
vars.actualLockupDynamicBalance = vars.balances[0];
vars.actualRecipientBalance = vars.balances[1];

Expand All @@ -340,7 +340,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
if (!vars.isDepleted && !vars.isSettled) {
// Load the pre-cancel asset balances.
vars.balances = getTokenBalances(
address(asset), Solarray.addresses(address(lockupDynamic), params.sender, params.recipient)
address(ASSET), Solarray.addresses(address(lockupDynamic), params.sender, params.recipient)
);
vars.initialLockupDynamicBalance = vars.balances[0];
vars.initialSenderBalance = vars.balances[1];
Expand All @@ -367,7 +367,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {

// Load the post-cancel asset balances.
vars.balances = getTokenBalances(
address(asset), Solarray.addresses(address(lockupDynamic), params.sender, params.recipient)
address(ASSET), Solarray.addresses(address(lockupDynamic), params.sender, params.recipient)
);
vars.actualLockupDynamicBalance = vars.balances[0];
vars.actualSenderBalance = vars.balances[1];
Expand Down
32 changes: 16 additions & 16 deletions test/fork/LockupLinear.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
CONSTRUCTOR
//////////////////////////////////////////////////////////////////////////*/

constructor(IERC20 asset_, address holder_) Fork_Test(asset_, holder_) { }
constructor(IERC20 asset, address holder) Fork_Test(asset, holder) { }

/*//////////////////////////////////////////////////////////////////////////
SET-UP FUNCTION
Expand All @@ -25,7 +25,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {

// Approve {SablierV2LockupLinear} to transfer the asset holder's assets.
// We use a low-level call to ignore reverts because the asset can have the missing return value bug.
(bool success,) = address(asset).call(abi.encodeCall(IERC20.approve, (address(lockupLinear), MAX_UINT256)));
(bool success,) = address(ASSET).call(abi.encodeCall(IERC20.approve, (address(lockupLinear), MAX_UINT256)));
success;
}

Expand Down Expand Up @@ -135,22 +135,22 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {

// Set the fuzzed protocol fee.
changePrank({ msgSender: users.admin });
comptroller.setProtocolFee({ asset: asset, newProtocolFee: params.protocolFee });
comptroller.setProtocolFee({ asset: ASSET, newProtocolFee: params.protocolFee });

// Make the holder the caller.
changePrank(holder);
changePrank(HOLDER);

/*//////////////////////////////////////////////////////////////////////////
CREATE
//////////////////////////////////////////////////////////////////////////*/

// Load the pre-create protocol revenues.
Vars memory vars;
vars.initialProtocolRevenues = lockupLinear.protocolRevenues(asset);
vars.initialProtocolRevenues = lockupLinear.protocolRevenues(ASSET);

// Load the pre-create asset balances.
vars.balances =
getTokenBalances(address(asset), Solarray.addresses(address(lockupLinear), params.broker.account));
getTokenBalances(address(ASSET), Solarray.addresses(address(lockupLinear), params.broker.account));
vars.initialLockupLinearBalance = vars.balances[0];
vars.initialBrokerBalance = vars.balances[1];

Expand All @@ -167,11 +167,11 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
vm.expectEmit({ emitter: address(lockupLinear) });
emit CreateLockupLinearStream({
streamId: vars.streamId,
funder: holder,
funder: HOLDER,
sender: params.sender,
recipient: params.recipient,
amounts: vars.createAmounts,
asset: asset,
asset: ASSET,
cancelable: true,
transferable: params.transferable,
range: params.range,
Expand All @@ -181,7 +181,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
// Create the stream.
lockupLinear.createWithRange(
LockupLinear.CreateWithRange({
asset: asset,
asset: ASSET,
broker: params.broker,
cancelable: true,
transferable: params.transferable,
Expand All @@ -195,7 +195,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
// Assert that the stream has been created.
LockupLinear.Stream memory actualStream = lockupLinear.getStream(vars.streamId);
assertEq(actualStream.amounts, Lockup.Amounts(vars.createAmounts.deposit, 0, 0));
assertEq(actualStream.asset, asset, "asset");
assertEq(actualStream.asset, ASSET, "asset");
assertEq(actualStream.cliffTime, params.range.cliff, "cliffTime");
assertEq(actualStream.endTime, params.range.end, "endTime");
assertEq(actualStream.isCancelable, true, "isCancelable");
Expand All @@ -217,7 +217,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
assertEq(vars.actualNextStreamId, vars.expectedNextStreamId, "post-create nextStreamId");

// Assert that the protocol fee has been recorded.
vars.actualProtocolRevenues = lockupLinear.protocolRevenues(asset);
vars.actualProtocolRevenues = lockupLinear.protocolRevenues(ASSET);
vars.expectedProtocolRevenues = vars.initialProtocolRevenues + vars.createAmounts.protocolFee;
assertEq(vars.actualProtocolRevenues, vars.expectedProtocolRevenues, "post-create protocolRevenues");

Expand All @@ -228,7 +228,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {

// Load the post-create asset balances.
vars.balances =
getTokenBalances(address(asset), Solarray.addresses(address(lockupLinear), holder, params.broker.account));
getTokenBalances(address(ASSET), Solarray.addresses(address(lockupLinear), HOLDER, params.broker.account));
vars.actualLockupLinearBalance = vars.balances[0];
vars.actualHolderBalance = vars.balances[1];
vars.actualBrokerBalance = vars.balances[2];
Expand Down Expand Up @@ -267,7 +267,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
if (params.withdrawAmount > 0) {
// Load the pre-withdraw asset balances.
vars.initialLockupLinearBalance = vars.actualLockupLinearBalance;
vars.initialRecipientBalance = asset.balanceOf(params.recipient);
vars.initialRecipientBalance = ASSET.balanceOf(params.recipient);

// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupLinear) });
Expand Down Expand Up @@ -301,7 +301,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {

// Load the post-withdraw asset balances.
vars.balances =
getTokenBalances(address(asset), Solarray.addresses(address(lockupLinear), params.recipient));
getTokenBalances(address(ASSET), Solarray.addresses(address(lockupLinear), params.recipient));
vars.actualLockupLinearBalance = vars.balances[0];
vars.actualRecipientBalance = vars.balances[1];

Expand All @@ -324,7 +324,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
if (!vars.isDepleted && !vars.isSettled) {
// Load the pre-cancel asset balances.
vars.balances = getTokenBalances(
address(asset), Solarray.addresses(address(lockupLinear), params.sender, params.recipient)
address(ASSET), Solarray.addresses(address(lockupLinear), params.sender, params.recipient)
);
vars.initialLockupLinearBalance = vars.balances[0];
vars.initialSenderBalance = vars.balances[1];
Expand All @@ -351,7 +351,7 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {

// Load the post-cancel asset balances.
vars.balances = getTokenBalances(
address(asset), Solarray.addresses(address(lockupLinear), params.sender, params.recipient)
address(ASSET), Solarray.addresses(address(lockupLinear), params.sender, params.recipient)
);
vars.actualLockupLinearBalance = vars.balances[0];
vars.actualSenderBalance = vars.balances[1];
Expand Down
8 changes: 4 additions & 4 deletions test/fork/assets/DAI.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import { LockupDynamic_Fork_Test } from "../LockupDynamic.t.sol";
import { LockupLinear_Fork_Test } from "../LockupLinear.t.sol";

/// @dev A typical 18-decimal ERC-20 asset with a normal total supply.
IERC20 constant asset = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);
address constant holder = 0x66F62574ab04989737228D18C3624f7FC1edAe14;
IERC20 constant ASSET = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);
address constant HOLDER = 0x66F62574ab04989737228D18C3624f7FC1edAe14;

contract DAI_LockupDynamic_Fork_Test is LockupDynamic_Fork_Test(asset, holder) { }
contract DAI_LockupDynamic_Fork_Test is LockupDynamic_Fork_Test(ASSET, HOLDER) { }

contract DAI_LockupLinear_Fork_Test is LockupLinear_Fork_Test(asset, holder) { }
contract DAI_LockupLinear_Fork_Test is LockupLinear_Fork_Test(ASSET, HOLDER) { }
Loading

0 comments on commit 8ca98d3

Please sign in to comment.