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

Feedback from PRB on V2.2 #883

Merged
merged 28 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ac1c4de
docs: improve wording in NatSpec
PaulRBerg Mar 29, 2024
55532b1
build: bump OpenZeppelin to v5.0.2
PaulRBerg Apr 4, 2024
1d693b9
docs: improve writing in NatSpec
PaulRBerg Apr 4, 2024
fb34afa
refactor: rename "streamingModel" to "sablierModel"
PaulRBerg Apr 5, 2024
91f1b7e
refactor: rename internal create function
PaulRBerg Apr 5, 2024
0119dc7
chore: singular Check, Effect, Interaction
PaulRBerg Apr 5, 2024
839607c
perf: close #878
PaulRBerg Apr 5, 2024
b510c03
refactor: reorder checks
PaulRBerg Apr 5, 2024
e88ef6e
test: remove unused import
smol-ninja Apr 6, 2024
bda5c85
test: include map symbol test for lockup tranched
smol-ninja Apr 6, 2024
5a500f8
revert: bring back GPL V3 license in mocks
PaulRBerg Apr 7, 2024
f6909be
refactor: update precompiles
PaulRBerg Apr 7, 2024
70b490f
test: reuse "createWithTimestamps" modifiers
PaulRBerg Apr 7, 2024
34bf2d4
test: revert max counts to 500
PaulRBerg Apr 7, 2024
7132b80
test: use lockup object
smol-ninja Apr 7, 2024
9371cee
style: correct bad formated natspec
andreivladbrg Apr 8, 2024
2ae73ab
refactor: rename "currentTime" to "blockTimestamp"
PaulRBerg Apr 8, 2024
49c7bea
docs: improve writing in NatSpec
PaulRBerg Apr 8, 2024
005f43d
refactor: order functions and fields alphabetically
PaulRBerg Apr 8, 2024
fff43d5
refactor: rename variable
PaulRBerg Apr 8, 2024
676fae1
docs: capitalize IDs
PaulRBerg Apr 8, 2024
351ae01
build: bump sphinx
smol-ninja Apr 9, 2024
188e244
test: remove ERC721Mock and use forge mock
andreivladbrg Apr 9, 2024
59cb669
test: update precompiles
smol-ninja Apr 9, 2024
c805841
chore: say "block timestamp" in comments
PaulRBerg Apr 8, 2024
13b6c09
refactor: update gas snapshot
PaulRBerg Apr 9, 2024
089564f
chore: add trailing slash
PaulRBerg Apr 9, 2024
12c6b32
build: bump PRBMath
PaulRBerg Apr 9, 2024
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 .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"func-name-mixedcase": "off",
"func-visibility": ["error", { "ignoreConstructors": true }],
"gas-custom-errors": "off",
"max-line-length": ["error", 123],
"max-line-length": ["error", 124],
"named-parameters-mapping": "warn",
"no-empty-blocks": "off",
"not-rely-on-time": "off",
Expand Down
Binary file modified bun.lockb
Binary file not shown.
6 changes: 3 additions & 3 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
auto_detect_solc = false
bytecode_hash = "none"
emv_version = "paris"
extra_output = ['storageLayout']
extra_output = ["storageLayout"]
ffi = true
fs_permissions = [
{ access = "read", path = "out-optimized" },
{ access = "read", path = "./out" },
{ access = "read-write", path = "./cache" }
{ access = "read", path = "./out-optimized" },
{ access = "read-write", path = "./cache" },
]
gas_reports = [
"SablierV2LockupDynamic",
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"url": "https://github.com/sablier-labs/v2-core/issues"
},
"dependencies": {
"@openzeppelin/contracts": "5.0.0",
"@openzeppelin/contracts": "5.0.2",
"@prb/math": "github:PaulRBerg/prb-math#a111d11"
},
"devDependencies": {
Expand Down Expand Up @@ -67,7 +67,7 @@
"gas:snapshot": "forge snapshot --mp \"./test/integration/**/*.sol\" --nmt \"test(Fuzz)?_RevertWhen_\\w{1,}?\"",
"gas:snapshot:optimized": "bun run build:optimized && FOUNDRY_PROFILE=test-optimized forge snapshot --mp \"./test/integration/**/*.sol\" --nmt \"test(Fork)?(Fuzz)?_RevertWhen_\\w{1,}?\"",
"lint": "bun run lint:sol && bun run prettier:check",
"lint:sol": "forge fmt --check && bun solhint \"{script,src,test}/**/*.sol\"",
"lint:sol": "forge fmt --check && bun solhint \"{precompiles,script,src,test}/**/*.sol\"",
"prepack": "bun install && bash ./shell/prepare-artifacts.sh",
"prettier:check": "prettier --check \"**/*.{json,md,svg,yml}\"",
"prettier:write": "prettier --write \"**/*.{json,md,svg,yml}\"",
Expand Down
19 changes: 10 additions & 9 deletions precompiles/Precompiles.sol

Large diffs are not rendered by default.

31 changes: 18 additions & 13 deletions script/Base.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,23 @@ contract BaseScript is Script, Sphinx {
/// @dev The Avalanche chain ID.
uint256 internal constant AVALANCHE_CHAIN_ID = 43_114;

/// @dev The project name for the Sphinx plugin.
string internal constant SPHINX_PROJECT_NAME = "test-test";

/// @dev Included to enable compilation of the script without a $MNEMONIC environment variable.
string internal constant TEST_MNEMONIC = "test test test test test test test test test test test junk";

/// @dev The project name for the Sphinx plugin.
string internal constant TEST_SPHINX_PROJECT_NAME = "test-test";

/// @dev Needed for the deterministic deployments.
bytes32 internal constant ZERO_SALT = bytes32(0);

/// @dev The address of the transaction broadcaster.
address internal broadcaster;

/// @dev The upper limit on the length of data structures to ensure that transactions stay within the
/// block gas limit.
uint256 internal maxCount;
/// @dev The upper limit on the length of segments to ensure that transactions stay within the block gas limit.
uint256 internal maxSegmentCount;

/// @dev The upper limit on the length of tranches to ensure that transactions stay within the block gas limit.
uint256 internal maxTrancheCount;

/// @dev Used to derive the broadcaster's address if $EOA is not defined.
string internal mnemonic;
Expand All @@ -52,13 +54,15 @@ contract BaseScript is Script, Sphinx {
mnemonic = vm.envOr({ name: "MNEMONIC", defaultValue: TEST_MNEMONIC });
(broadcaster,) = deriveRememberKey({ mnemonic: mnemonic, index: 0 });
}
sphinxProjectName = vm.envOr({ name: "SPHINX_PROJECT_NAME", defaultValue: SPHINX_PROJECT_NAME });
sphinxProjectName = vm.envOr({ name: "SPHINX_PROJECT_NAME", defaultValue: TEST_SPHINX_PROJECT_NAME });

// Sets `maxCount` to 300 for Avalanche, and 500 for all other chains.
// Avalanche has a lower block gas limit than most other chains.
if (block.chainid == AVALANCHE_CHAIN_ID) {
maxCount = 300;
maxSegmentCount = 300;
maxTrancheCount = 298;
PaulRBerg marked this conversation as resolved.
Show resolved Hide resolved
} else {
maxCount = 500;
maxSegmentCount = 500;
maxTrancheCount = 500;
}
}

Expand All @@ -68,10 +72,11 @@ contract BaseScript is Script, Sphinx {
vm.stopBroadcast();
}

/// @dev Configures the Sphinx plugin to use Sphinx managed deployment for smart contracts.
/// @dev Configures the Sphinx plugin to manage the deployment of the contracts.
/// Refer to https://github.com/sphinx-labs/sphinx/tree/main/docs.
///
/// CLI example:
/// - bun sphinx propose script/DeployCore.s.sol --networks testnets --sig "runSphinx(address)" $ADMIN
/// bun sphinx propose script/DeployCore.s.sol --networks testnets --sig "runSphinx(address)" $ADMIN
function configureSphinx() public override {
sphinxConfig.mainnets = ["arbitrum", "avalanche", "base", "bnb", "gnosis", "ethereum", "optimism", "polygon"];
sphinxConfig.orgId = vm.envOr({ name: "SPHINX_ORG_ID", defaultValue: TEST_MNEMONIC });
Expand All @@ -88,7 +93,7 @@ contract BaseScript is Script, Sphinx {
/// - The salt format is "ChainID <chainid>, Version <version>".
/// - The version is obtained from `package.json` using the `ffi` cheatcode:
/// https://book.getfoundry.sh/cheatcodes/ffi
/// - Requires `jq` CLI tool installed: https://jqlang.github.io/jq/
/// - Requires the `jq` CLI installed: https://jqlang.github.io/jq/
function constructCreate2Salt() public returns (bytes32) {
string memory chainId = block.chainid.toString();
string[] memory inputs = new string[](4);
Expand Down
8 changes: 4 additions & 4 deletions script/DeployCore.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { BaseScript } from "./Base.s.sol";
/// 3. {SablierV2LockupLinear}
/// 4. {SablierV2LockupTranched}
contract DeployCore is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(address initialAdmin)
public
virtual
Expand All @@ -30,7 +30,7 @@ contract DeployCore is BaseScript {
(lockupDynamic, lockupLinear, lockupTranched, nftDescriptor) = _run(initialAdmin);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(address initialAdmin)
public
virtual
Expand All @@ -55,8 +55,8 @@ contract DeployCore is BaseScript {
)
{
nftDescriptor = new SablierV2NFTDescriptor();
lockupDynamic = new SablierV2LockupDynamic(initialAdmin, nftDescriptor, maxCount);
lockupDynamic = new SablierV2LockupDynamic(initialAdmin, nftDescriptor, maxSegmentCount);
lockupLinear = new SablierV2LockupLinear(initialAdmin, nftDescriptor);
lockupTranched = new SablierV2LockupTranched(initialAdmin, nftDescriptor, maxCount);
lockupTranched = new SablierV2LockupTranched(initialAdmin, nftDescriptor, maxTrancheCount);
}
}
8 changes: 4 additions & 4 deletions script/DeployCore2.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { BaseScript } from "./Base.s.sol";
/// 2. {SablierV2LockupLinear}
/// 3. {SablierV2LockupTranched}
contract DeployCore2 is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor nftDescriptor
Expand All @@ -31,7 +31,7 @@ contract DeployCore2 is BaseScript {
(lockupDynamic, lockupLinear, lockupTranched) = _run(initialAdmin, nftDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor nftDescriptor
Expand Down Expand Up @@ -59,8 +59,8 @@ contract DeployCore2 is BaseScript {
SablierV2LockupTranched lockupTranched
)
{
lockupDynamic = new SablierV2LockupDynamic(initialAdmin, nftDescriptor, maxCount);
lockupDynamic = new SablierV2LockupDynamic(initialAdmin, nftDescriptor, maxSegmentCount);
lockupLinear = new SablierV2LockupLinear(initialAdmin, nftDescriptor);
lockupTranched = new SablierV2LockupTranched(initialAdmin, nftDescriptor, maxCount);
lockupTranched = new SablierV2LockupTranched(initialAdmin, nftDescriptor, maxTrancheCount);
}
}
8 changes: 4 additions & 4 deletions script/DeployDeterministicCore.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { BaseScript } from "./Base.s.sol";
///
/// @dev Reverts if any contract has already been deployed.
contract DeployDeterministicCore is BaseScript {
/// @dev Deploy using Forge.
/// @dev Deploy via Forge.
function runBroadcast(address initialAdmin)
public
virtual
Expand All @@ -32,7 +32,7 @@ contract DeployDeterministicCore is BaseScript {
(lockupDynamic, lockupLinear, lockupTranched, nftDescriptor) = _run(initialAdmin);
}

/// @dev Deploy using Sphinx.
/// @dev Deploy via Sphinx.
function runSphinx(address initialAdmin)
public
virtual
Expand All @@ -58,8 +58,8 @@ contract DeployDeterministicCore is BaseScript {
{
bytes32 salt = constructCreate2Salt();
nftDescriptor = new SablierV2NFTDescriptor{ salt: salt }();
lockupDynamic = new SablierV2LockupDynamic{ salt: salt }(initialAdmin, nftDescriptor, maxCount);
lockupDynamic = new SablierV2LockupDynamic{ salt: salt }(initialAdmin, nftDescriptor, maxSegmentCount);
lockupLinear = new SablierV2LockupLinear{ salt: salt }(initialAdmin, nftDescriptor);
lockupTranched = new SablierV2LockupTranched{ salt: salt }(initialAdmin, nftDescriptor, maxCount);
lockupTranched = new SablierV2LockupTranched{ salt: salt }(initialAdmin, nftDescriptor, maxTrancheCount);
}
}
8 changes: 4 additions & 4 deletions script/DeployDeterministicCore2.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { BaseScript } from "./Base.s.sol";
///
/// @dev Reverts if any contract has already been deployed.
contract DeployDeterministicCore2 is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor nftDescriptor
Expand All @@ -33,7 +33,7 @@ contract DeployDeterministicCore2 is BaseScript {
(lockupDynamic, lockupLinear, lockupTranched) = _run(initialAdmin, nftDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor nftDescriptor
Expand Down Expand Up @@ -62,8 +62,8 @@ contract DeployDeterministicCore2 is BaseScript {
)
{
bytes32 salt = constructCreate2Salt();
lockupDynamic = new SablierV2LockupDynamic{ salt: salt }(initialAdmin, nftDescriptor, maxCount);
lockupDynamic = new SablierV2LockupDynamic{ salt: salt }(initialAdmin, nftDescriptor, maxSegmentCount);
lockupLinear = new SablierV2LockupLinear{ salt: salt }(initialAdmin, nftDescriptor);
lockupTranched = new SablierV2LockupTranched{ salt: salt }(initialAdmin, nftDescriptor, maxCount);
lockupTranched = new SablierV2LockupTranched{ salt: salt }(initialAdmin, nftDescriptor, maxTrancheCount);
}
}
6 changes: 3 additions & 3 deletions script/DeployDeterministicLockupDynamic.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { BaseScript } from "./Base.s.sol";
/// @notice Deploys {SablierV2LockupDynamic} at a deterministic address across chains.
/// @dev Reverts if the contract has already been deployed.
contract DeployDeterministicLockupDynamic is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -22,7 +22,7 @@ contract DeployDeterministicLockupDynamic is BaseScript {
lockupDynamic = _run(initialAdmin, initialNFTDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -43,6 +43,6 @@ contract DeployDeterministicLockupDynamic is BaseScript {
returns (SablierV2LockupDynamic lockupDynamic)
{
bytes32 salt = constructCreate2Salt();
lockupDynamic = new SablierV2LockupDynamic{ salt: salt }(initialAdmin, initialNFTDescriptor, maxCount);
lockupDynamic = new SablierV2LockupDynamic{ salt: salt }(initialAdmin, initialNFTDescriptor, maxSegmentCount);
}
}
4 changes: 2 additions & 2 deletions script/DeployDeterministicLockupLinear.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { BaseScript } from "./Base.s.sol";
/// @dev Deploys {SablierV2LockupLinear} at a deterministic address across chains.
/// @dev Reverts if the contract has already been deployed.
contract DeployDeterministicLockupLinear is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -22,7 +22,7 @@ contract DeployDeterministicLockupLinear is BaseScript {
lockupLinear = _run(initialAdmin, initialNFTDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand Down
6 changes: 3 additions & 3 deletions script/DeployDeterministicLockupTranched.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { BaseScript } from "./Base.s.sol";
/// @dev Deploys {SablierV2LockupTranched} at a deterministic address across chains.
/// @dev Reverts if the contract has already been deployed.
contract DeployDeterministicLockupTranched is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -22,7 +22,7 @@ contract DeployDeterministicLockupTranched is BaseScript {
lockupTranched = _run(initialAdmin, initialNFTDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -43,6 +43,6 @@ contract DeployDeterministicLockupTranched is BaseScript {
returns (SablierV2LockupTranched lockupTranched)
{
bytes32 salt = constructCreate2Salt();
lockupTranched = new SablierV2LockupTranched{ salt: salt }(initialAdmin, initialNFTDescriptor, maxCount);
lockupTranched = new SablierV2LockupTranched{ salt: salt }(initialAdmin, initialNFTDescriptor, maxTrancheCount);
}
}
4 changes: 2 additions & 2 deletions script/DeployDeterministicNFTDescriptor.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import { BaseScript } from "./Base.s.sol";
/// @dev Deploys {SablierV2NFTDescriptor} at a deterministic address across chains.
/// @dev Reverts if the contract has already been deployed.
contract DeployDeterministicNFTDescriptor is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast() public virtual broadcast returns (SablierV2NFTDescriptor nftDescriptor) {
nftDescriptor = _run();
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx() public virtual sphinx returns (SablierV2NFTDescriptor nftDescriptor) {
nftDescriptor = _run();
}
Expand Down
6 changes: 3 additions & 3 deletions script/DeployLockupDynamic.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { SablierV2LockupDynamic } from "../src/SablierV2LockupDynamic.sol";
import { BaseScript } from "./Base.s.sol";

contract DeployLockupDynamic is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -20,7 +20,7 @@ contract DeployLockupDynamic is BaseScript {
lockupDynamic = _run(initialAdmin, initialNFTDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -40,6 +40,6 @@ contract DeployLockupDynamic is BaseScript {
internal
returns (SablierV2LockupDynamic lockupDynamic)
{
lockupDynamic = new SablierV2LockupDynamic(initialAdmin, initialNFTDescriptor, maxCount);
lockupDynamic = new SablierV2LockupDynamic(initialAdmin, initialNFTDescriptor, maxSegmentCount);
}
}
4 changes: 2 additions & 2 deletions script/DeployLockupLinear.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { SablierV2LockupLinear } from "../src/SablierV2LockupLinear.sol";
import { BaseScript } from "./Base.s.sol";

contract DeployLockupLinear is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -20,7 +20,7 @@ contract DeployLockupLinear is BaseScript {
lockupLinear = _run(initialAdmin, initialNFTDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand Down
6 changes: 3 additions & 3 deletions script/DeployLockupTranched.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { SablierV2LockupTranched } from "../src/SablierV2LockupTranched.sol";
import { BaseScript } from "./Base.s.sol";

contract DeployLockupTranched is BaseScript {
/// @dev Deploy using Forge CLI.
/// @dev Deploy via Forge.
function runBroadcast(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -20,7 +20,7 @@ contract DeployLockupTranched is BaseScript {
lockupTranched = _run(initialAdmin, initialNFTDescriptor);
}

/// @dev Deploy using Sphinx CLI.
/// @dev Deploy via Sphinx.
function runSphinx(
address initialAdmin,
ISablierV2NFTDescriptor initialNFTDescriptor
Expand All @@ -40,6 +40,6 @@ contract DeployLockupTranched is BaseScript {
internal
returns (SablierV2LockupTranched lockupTranched)
{
lockupTranched = new SablierV2LockupTranched(initialAdmin, initialNFTDescriptor, maxCount);
lockupTranched = new SablierV2LockupTranched(initialAdmin, initialNFTDescriptor, maxTrancheCount);
}
}
Loading
Loading