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

HUBS-224 crowdsale with individual locking staking periods #170

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
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
14 changes: 7 additions & 7 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ PRICEFEED_ADDRESS=0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6
TERMS_ACCEPTED_PERMISSIONER_ADDRESS=0x8A791620dd6260079BF849Dc5567aDC3F2FdC318

TOKENIZER_ADDRESS=0xB7f8BC63BbcaD18155201308C8f3540b07f84F5e
STAKED_LOCKING_CROWDSALE_ADDRESS=0x0B306BF915C4d645ff596e518fAf3F9669b97016
#iptoken implementation=0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82

USDC6_ADDRESS=0x68B1D87F95878fE05B998F19b66F4baba5De1aed
WETH_ADDRESS=0x4ed7c70F96B99c776995fB64377f0d4aB3B0e1C1
PLAIN_CROWDSALE_ADDRESS=0x7a2088a1bFc9d81c55368AE168C2C02570cB814F
#timelocked token implementation=0x0B306BF915C4d645ff596e518fAf3F9669b97016
STAKED_LOCKING_CROWDSALE_ADDRESS=0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1

#this is created during the tokenizer deployment
IPTOKEN_IMPLEMENTATION_ADDRESS=0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82
USDC6_ADDRESS=0x3Aa5ebB10DC797CAC828524e59A333d0A371443c
WETH_ADDRESS=0x322813Fd9A801c5507c9de605d63CEA4f2CE6c44
PLAIN_CROWDSALE_ADDRESS=0x09635F643e140090A9A8Dcd712eD6285858ceBef

#these are generated when running the fixture scripts
IPTS_ADDRESS=0x8dAF17A20c9DBA35f005b6324F493785D239719d
LOCKED_IPTS_ADDRESS=0x16eBC21B3d38Db5e3EE1a022bEBA8Ec87D4CDbe6
LOCKED_IPTS_ADDRESS=0x24B3c7704709ed1491473F30393FFc93cFB0FC34
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ Deprecated after migrating to Defender 2 (was 0x3D30452c48F2448764d5819a9A2b684A
| Terms Permissioner | 0xC05D649368d8A5e2E98CAa205d47795de5fCB599 | <a href="https://sepolia.etherscan.io/address/0xC05D649368d8A5e2E98CAa205d47795de5fCB599#code" target="_blank"><img width="200" height="45" src="https://badges.thirdweb.com/contract?address=0xC05D649368d8A5e2E98CAa205d47795de5fCB599&theme=dark&chainId=1" alt="View contract" /></a> |
| Tokenizer | 0xca63411FF5187431028d003eD74B57531408d2F9 | <a href="https://sepolia.etherscan.io/address/0xca63411FF5187431028d003eD74B57531408d2F9#code" target="_blank"><img width="200" height="45" src="https://badges.thirdweb.com/contract?address=0xca63411FF5187431028d003eD74B57531408d2F9&theme=dark&chainId=1" alt="View contract" /></a> |
| Crowdsale | 0x8cA737E2cdaE1Ceb332bEf7ba9eA711a3a2f8037 | <a href="https://sepolia.etherscan.io/address/0x8cA737E2cdaE1Ceb332bEf7ba9eA711a3a2f8037#code" target="_blank"><img width="200" height="45" src="https://badges.thirdweb.com/contract?address=0x8cA737E2cdaE1Ceb332bEf7ba9eA711a3a2f8037&theme=dark&chainId=1" alt="View contract" /></a> |
| Staked Crowdsale | 0xd1cE2EA7d3b0C9cAB025A4aD762FC00315141ad7 | <a href="https://sepolia.etherscan.io/address/0xd1cE2EA7d3b0C9cAB025A4aD762FC00315141ad7#code" target="_blank"><img width="200" height="45" src="https://badges.thirdweb.com/contract?address=0xd1cE2EA7d3b0C9cAB025A4aD762FC00315141ad7&theme=dark&chainId=1" alt="View contract" /></a> |
| Locking Crowdsale | 0x0Da77f361bB56f065Aa21647d885685eb7cAE10F | <a href="https://sepolia.etherscan.io/address/0x0Da77f361bB56f065Aa21647d885685eb7cAE10F#code" target="_blank"><img width="200" height="45" src="https://badges.thirdweb.com/contract?address=0x0Da77f361bB56f065Aa21647d885685eb7cAE10F&theme=dark&chainId=1" alt="View contract" /></a> |
| Staked Crowdsale | 0x2d309CF13dC3872f9c9B1B06Ebf6F60caDe08d55 | <a href="https://sepolia.etherscan.io/address/0x2d309CF13dC3872f9c9B1B06Ebf6F60caDe08d55#code" target="_blank"><img width="200" height="45" src="https://badges.thirdweb.com/contract?address=0x2d309CF13dC3872f9c9B1B06Ebf6F60caDe08d55&theme=dark&chainId=11155111" alt="View contract" /></a> |

timelocked token implementation=0xF8F79c1E02387b0Fc9DE0945cD9A2c06F127D851

previous SLCS: https://sepolia.etherscan.io/address/0xd1cE2EA7d3b0C9cAB025A4aD762FC00315141ad7#code

#### Subgraphs

Expand Down Expand Up @@ -204,6 +209,9 @@ The crowdsale computation model can be tried out here: <https://docs.google.com/
Deploying and verifying a single contract without the help of any script
`forge create --rpc-url $RPC_URL --private-key $PRIVATE_KEY --chain 5 --etherscan-api-key $ETHERSCAN_API_KEY --verify src/crowdsale/StakedLockingCrowdSale.sol:StakedLockingCrowdSale`

Verifying a the staked crowdsale crowdsale
`forge verify-contract --chain-id=11155111 --etherscan-api-key=$ETHERSCAN_API_KEY --constructor-args $(cast abi-encode "constructor(address)" 0xF8F79c1E02387b0Fc9DE0945cD9A2c06F127D851) 0x7eeb7113f90893fb95c6666e3930235850f2bc6A src/crowdsale/StakedLockingCrowdSale.sol:StakedLockingCrowdSale`

### Deploying (vested) test tokens

To test staked / vested token interactions, you need some test tokens. Here are 2 convenient script to get them running:
Expand Down
6 changes: 5 additions & 1 deletion script/DeployTokenizer.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { BioPriceFeed } from "../src/BioPriceFeed.sol";
import { IPermissioner, TermsAcceptedPermissioner } from "../src/Permissioner.sol";
import { CrowdSale } from "../src/crowdsale/CrowdSale.sol";
import { StakedLockingCrowdSale } from "../src/crowdsale/StakedLockingCrowdSale.sol";
import { TimelockedToken } from "../src/TimelockedToken.sol";

contract DeployTokenizerInfrastructure is Script {
function run() public {
Expand All @@ -27,13 +28,16 @@ contract DeployTokenizerInfrastructure is Script {
tokenizer.setIPTokenImplementation(initialIpTokenImplementation);

CrowdSale crowdSale = new CrowdSale();
StakedLockingCrowdSale stakedLockingCrowdSale = new StakedLockingCrowdSale();
//this allows the default TimelockedToken implementation to be verified on chain explorers
TimelockedToken timelockedTokenImplementation = new TimelockedToken();
StakedLockingCrowdSale stakedLockingCrowdSale = new StakedLockingCrowdSale(timelockedTokenImplementation);
vm.stopBroadcast();

console.log("TERMS_ACCEPTED_PERMISSIONER_ADDRESS=%s", address(permissioner));
console.log("TOKENIZER_ADDRESS=%s", address(tokenizer));
console.log("CROWDSALE_ADDRESS=%s", address(crowdSale));
console.log("STAKED_LOCKING_CROWDSALE_ADDRESS=%s", address(stakedLockingCrowdSale));
console.log("timelocked token implementation=%s", address(timelockedTokenImplementation));
console.log("initial IP Token implementation=%s", address(initialIpTokenImplementation));
}
}
Expand Down
12 changes: 5 additions & 7 deletions script/dev/CrowdSale.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,19 @@ contract DeployCrowdSale is CommonScript {
}
}

/**
* @title deploy crowdSale
* @author
*/
contract DeployStakedCrowdSale is CommonScript {
function run() public {
prepareAddresses();
vm.startBroadcast(deployer);
StakedLockingCrowdSale stakedLockingCrowdSale = new StakedLockingCrowdSale();

TimelockedToken lockingCrowdsaleImplementation = new TimelockedToken();
StakedLockingCrowdSale stakedLockingCrowdSale = new StakedLockingCrowdSale(lockingCrowdsaleImplementation);

TokenVesting vestedDaoToken = TokenVesting(vm.envAddress("VDAO_TOKEN_ADDRESS"));
vestedDaoToken.grantRole(vestedDaoToken.ROLE_CREATE_SCHEDULE(), address(stakedLockingCrowdSale));
stakedLockingCrowdSale.trustVestingContract(vestedDaoToken);
vm.stopBroadcast();

console.log("timelocked token implementation=%s", address(lockingCrowdsaleImplementation));
console.log("STAKED_LOCKING_CROWDSALE_ADDRESS=%s", address(stakedLockingCrowdSale));
}
}
Expand Down Expand Up @@ -154,7 +152,7 @@ contract FixtureStakedCrowdSale is FixtureCrowdSale {
function startSale() internal override returns (uint256 saleId) {
Sale memory _sale = prepareRun();
vm.startBroadcast(bob);
saleId = _slCrowdSale.startSale(_sale, daoToken, vestedDaoToken, 1e18, 7 days);
saleId = _slCrowdSale.startSale(_sale, daoToken, vestedDaoToken, 1e18, 7 days, 7 days);
vm.stopBroadcast();
}

Expand Down
2 changes: 1 addition & 1 deletion script/dev/SignTermsMessage.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "forge-std/console.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

contract SignTermsMessage is Script {
function run() public {
function run() public view {
uint256 pk = vm.envUint("PRIVATE_KEY");
string memory terms =
"As an IP token holder of IPNFT #10, I accept all terms that I've read here: ipfs://bafkreigk5dvqblnkdniges6ft5kmuly47ebw4vho6siikzmkaovq6sjstq\n\nChain Id: 31337\nVersion: 1";
Expand Down
2 changes: 1 addition & 1 deletion script/dev/Tokenizer.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract DeployTokenizer is CommonScript {

vm.stopBroadcast();
console.log("TOKENIZER_ADDRESS=%s", address(tokenizer));
console.log("IPTOKEN_IMPLEMENTATION_ADDRESS=%s", address(initialIpTokenImplementation));
console.log("iptoken implementation=%s", address(initialIpTokenImplementation));
}
}

Expand Down
25 changes: 0 additions & 25 deletions script/prod/RolloutV23Sale.sol

This file was deleted.

54 changes: 54 additions & 0 deletions script/prod/RolloutV25Sale.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;

import "forge-std/Script.sol";
import "forge-std/console.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import { IPNFT } from "../../src/IPNFT.sol";
import { IPermissioner, TermsAcceptedPermissioner } from "../../src/Permissioner.sol";
import { StakedLockingCrowdSale } from "../../src/crowdsale/StakedLockingCrowdSale.sol";
import { LockingCrowdSale } from "../../src/crowdsale/LockingCrowdSale.sol";
import { TimelockedToken } from "../../src/TimelockedToken.sol";
import { TokenVesting } from "@moleculeprotocol/token-vesting/TokenVesting.sol";

TimelockedToken constant timelockedTokenImplementation = TimelockedToken(0xF8F79c1E02387b0Fc9DE0945cD9A2c06F127D851);
address constant moleculeDevMultisig = 0x9d5a6ae551f1117946FF6e0e86ef9A1B20C90Cb0;
Comment on lines +14 to +15
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Hardcoded addresses need to be updated for mainnet deployment

The hardcoded addresses in RolloutV25Sale.sol are currently set for the Sepolia testnet environment, as evidenced by:

  • The TimelockedToken implementation address (0xF8F79c1E02387b0Fc9DE0945cD9A2c06F127D851) is listed in README.md as a Sepolia deployment
  • There's a commented mainnet address in the same file showing this is meant to be updated
  • The repository's README.md confirms these are Sepolia testnet deployments

Please update these addresses with the appropriate mainnet values before deployment:

  • Replace TimelockedToken implementation address with the mainnet deployment
  • Update moleculeDevMultisig to the commented mainnet address (0xCfA0F84660fB33bFd07C369E5491Ab02C449f71B)
🔗 Analysis chain

Validate hardcoded contract addresses

The TimelockedToken implementation and multisig addresses should be validated before deployment.


Let me try to verify the addresses by searching for their usage in the codebase to understand the context better.


Let me search for more context about the TimelockedToken contract and its implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify contract addresses and implementations

# Check if addresses are valid contracts
for addr in "0xF8F79c1E02387b0Fc9DE0945cD9A2c06F127D851" "0x9d5a6ae551f1117946FF6e0e86ef9A1B20C90Cb0"; do
  echo "Checking $addr..."
  # Get contract bytecode to verify it's a contract
  cast code $addr
  # Get implementation if it's a proxy
  cast storage $addr 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc
done

Length of output: 1187


Script:

#!/bin/bash
# Search for the TimelockedToken implementation address
echo "Searching for TimelockedToken implementation address..."
rg -A 3 "0xF8F79c1E02387b0Fc9DE0945cD9A2c06F127D851"

echo -e "\nSearching for moleculeDevMultisig address..."
rg -A 3 "0x9d5a6ae551f1117946FF6e0e86ef9A1B20C90Cb0"

echo -e "\nSearching for TimelockedToken contract definition..."
ast-grep --pattern 'contract TimelockedToken {
  $$$
}'

Length of output: 1750


Script:

#!/bin/bash
# Search for TimelockedToken interface or contract
echo "Searching for TimelockedToken interface or contract..."
fd -e sol -x cat {} | grep -A 10 "contract TimelockedToken"

echo -e "\nSearching for imports of TimelockedToken..."
rg "import.*TimelockedToken"

echo -e "\nChecking if this is a test environment..."
rg "sepolia|testnet|test environment" README.md

Length of output: 7524


//mainnet 0xCfA0F84660fB33bFd07C369E5491Ab02C449f71B;

contract RolloutV25Sale is Script {
function run() public {

TokenVesting vesting = TokenVesting(0x8f80d1183CD983B01B0C9AC6777cC732Ec9800de); //Moldao

vm.startBroadcast();
StakedLockingCrowdSale stakedLockingCrowdSale = new StakedLockingCrowdSale(timelockedTokenImplementation);
vesting.grantRole(vesting.ROLE_CREATE_SCHEDULE(), address(stakedLockingCrowdSale));
//stakedLockingCrowdSale.trustLockingContract(IERC20());
stakedLockingCrowdSale.trustVestingContract(vesting);
// stakedLockingCrowdSale.transferOwnership(moleculeDevMultisig);
Comment on lines +27 to +29
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Based on the search results, I can now provide a final response:

Critical: Trust relationships and ownership transfer must be properly configured

The commented lines in RolloutV25Sale.sol require attention:

  • The trustVestingContract call is correctly implemented and matches the contract's requirements as seen in StakedLockingCrowdSale.sol
  • The commented trustLockingContract call is using an incorrect parameter (empty IERC20)
  • The commented transferOwnership is a required security step for production deployment

Required actions:

  • Implement trustLockingContract with the correct token address
  • Uncomment and implement transferOwnership to ensure proper contract management
  • Verify the vesting contract has the correct ROLE_CREATE_SCHEDULE role before deployment
🔗 Analysis chain

Critical: Implement contract trust relationships

The commented lines indicate missing trust relationships between contracts:

//stakedLockingCrowdSale.trustLockingContract(IERC20());
stakedLockingCrowdSale.trustVestingContract(vesting);
//stakedLockingCrowdSale.transferOwnership(moleculeDevMultisig);

These relationships are crucial for the crowdsale functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify vesting contract configuration

# Check if vesting contract exists and has correct roles
echo "Checking vesting contract at 0x8f80d1183CD983B01B0C9AC6777cC732Ec9800de..."
cast call 0x8f80d1183CD983B01B0C9AC6777cC732Ec9800de "hasRole(bytes32,address)" \
  $(cast --format-bytes32-string "ROLE_CREATE_SCHEDULE") \
  0x2d309CF13dC3872f9c9B1B06Ebf6F60caDe08d55

Length of output: 526


Script:

#!/bin/bash
# Let's analyze the contract relationships and dependencies

# Find all references to trustLockingContract and trustVestingContract
echo "Searching for trust relationships..."
rg -A 3 "trustLockingContract|trustVestingContract" --type solidity

# Find ownership transfer patterns
echo -e "\nSearching for ownership transfers..."
rg -A 3 "transferOwnership" --type solidity

# Find contract interfaces and implementations
echo -e "\nSearching for related contract definitions..."
rg -B 2 "contract StakedLockingCrowdSale|contract TokenVesting" --type solidity

Length of output: 573


Script:

#!/bin/bash
# Let's analyze the contract relationships and dependencies without file type filter

# Find all references to trustLockingContract and trustVestingContract
echo "Searching for trust relationships..."
rg -A 3 "trustLockingContract|trustVestingContract"

# Find ownership transfer patterns
echo -e "\nSearching for ownership transfers..."
rg -A 3 "transferOwnership"

# Find contract interfaces and implementations
echo -e "\nSearching for related contract definitions..."
rg -B 2 "contract StakedLockingCrowdSale|contract TokenVesting"

# List all Solidity files to understand the contract structure
echo -e "\nListing Solidity files..."
fd -e sol

Length of output: 12463

vm.stopBroadcast();

console.log("STAKED_LOCKING_CROWDSALE_ADDRESS=%s", address(stakedLockingCrowdSale));
console.log("timelocked token implementation=%s", address(timelockedTokenImplementation));
// 0x7c36c64DA1c3a2065074caa9C48e7648FB733aAB
// vestedDaoToken.grantRole(vestedDaoToken.ROLE_CREATE_SCHEDULE(), address(stakedLockingCrowdSale));
// stakedLockingCrowdSale.trustVestingContract(vestedDaoToken);
}
Comment on lines +34 to +37
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Uncomment and implement role management

The commented code appears to handle critical role assignments:

// vestedDaoToken.grantRole(vestedDaoToken.ROLE_CREATE_SCHEDULE(), address(stakedLockingCrowdSale));
// stakedLockingCrowdSale.trustVestingContract(vestedDaoToken);

This should either be:

  1. Implemented properly if required for the crowdsale functionality
  2. Removed if no longer needed
  3. Documented why it's intentionally skipped

Also applies to: 47-49

}

contract RolloutV25LockingSale is Script {
function run() public {

vm.startBroadcast();
LockingCrowdSale lockingCrowdsale = new LockingCrowdSale(timelockedTokenImplementation);
//lockingCrowdsale.transferOwnership(moleculeDevMultisig);
vm.stopBroadcast();
Comment on lines +44 to +46
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Implement ownership transfer

The ownership transfer to the multisig is commented out:

//lockingCrowdsale.transferOwnership(moleculeDevMultisig);

This is a security risk as the deployer will retain control.


console.log("LOCKING_CROWDSALE_ADDRESS=%s", address(lockingCrowdsale));
console.log("timelocked token implementation=%s", address(timelockedTokenImplementation));
// 0x7c36c64DA1c3a2065074caa9C48e7648FB733aAB
// vestedDaoToken.grantRole(vestedDaoToken.ROLE_CREATE_SCHEDULE(), address(stakedLockingCrowdSale));
// stakedLockingCrowdSale.trustVestingContract(vestedDaoToken);
}
}
8 changes: 8 additions & 0 deletions setupLocal.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ set +a
fixtures=false
extrafixtures=false

show_help() {
echo "Usage: setupLocal.sh [OPTION]"
echo "Sets up the local environment for the IPNFT contracts."
echo "Options:"
echo " -f also runs basic fixture scripts"
echo " -x also runs extra fixture scripts (crowdsales)"
}

# Parse command-line options
while getopts "fx" opt; do
case ${opt} in
Expand Down
20 changes: 18 additions & 2 deletions src/crowdsale/LockingCrowdSale.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol";
Expand All @@ -12,6 +13,7 @@ import { CrowdSale, Sale } from "./CrowdSale.sol";
error UnsupportedInitializer();
error InvalidDuration();


/**
* @title LockingCrowdSale
* @author molecule.to
Expand All @@ -25,16 +27,30 @@ contract LockingCrowdSale is CrowdSale {
/// @notice map from token address to reusable TimelockedToken contracts
mapping(address => TimelockedToken) public lockingContracts;

address immutable lockingTokenImplementation = address(new TimelockedToken());
address immutable public TIMELOCKED_TOKEN_IMPLEMENTATION;

event Started(uint256 indexed saleId, address indexed issuer, Sale sale, TimelockedToken lockingToken, uint256 lockingDuration, uint16 feeBp);
event LockingContractCreated(TimelockedToken indexed lockingContract, IERC20Metadata indexed underlyingToken);

constructor(TimelockedToken _timelockedTokenImplementation) {
TIMELOCKED_TOKEN_IMPLEMENTATION = address(_timelockedTokenImplementation);
}
Comment on lines +35 to +37
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add address validation in constructor.

The constructor should validate that the provided TimelockedToken implementation address is not zero to prevent potential deployment issues.

 constructor(TimelockedToken _timelockedTokenImplementation) {
+    require(address(_timelockedTokenImplementation) != address(0), "Invalid implementation address");
     TIMELOCKED_TOKEN_IMPLEMENTATION = address(_timelockedTokenImplementation);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
constructor(TimelockedToken _timelockedTokenImplementation) {
TIMELOCKED_TOKEN_IMPLEMENTATION = address(_timelockedTokenImplementation);
}
constructor(TimelockedToken _timelockedTokenImplementation) {
require(address(_timelockedTokenImplementation) != address(0), "Invalid implementation address");
TIMELOCKED_TOKEN_IMPLEMENTATION = address(_timelockedTokenImplementation);
}


/// @dev disable parent sale starting functions
function startSale(Sale calldata) public pure override returns (uint256) {
revert UnsupportedInitializer();
}

/**
* @notice allows the owner to trust a timelocked token contract for a specific underlying token so it's not registered again.
*
* @param token the underlying token
* @param _timelockedToken the timelocked token contract to trust
*/
function trustLockingContract(IERC20 token, TimelockedToken _timelockedToken) public onlyOwner {
lockingContracts[address(token)] = _timelockedToken;
}

/**
* @notice allows anyone to create a timelocked token that's controlled by this sale contract
* helpful if you want to reuse the timelocked token for your own custom schedules
Expand Down Expand Up @@ -114,7 +130,7 @@ contract LockingCrowdSale is CrowdSale {
* @return lockedTokenContract address of the new timelocked token contract
*/
function _makeNewLockedTokenContract(IERC20Metadata auctionToken) private returns (TimelockedToken lockedTokenContract) {
lockedTokenContract = TimelockedToken(Clones.clone(lockingTokenImplementation));
lockedTokenContract = TimelockedToken(Clones.clone(TIMELOCKED_TOKEN_IMPLEMENTATION));
lockedTokenContract.initialize(auctionToken);
emit LockingContractCreated(lockedTokenContract, auctionToken);
}
Comment on lines +133 to 136
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for external calls in _makeNewLockedTokenContract.

The initialization call could fail but there's no error handling. Consider adding try-catch to handle potential initialization failures gracefully.

 function _makeNewLockedTokenContract(IERC20Metadata auctionToken) private returns (TimelockedToken lockedTokenContract) {
     lockedTokenContract = TimelockedToken(Clones.clone(TIMELOCKED_TOKEN_IMPLEMENTATION));
-    lockedTokenContract.initialize(auctionToken);
+    try lockedTokenContract.initialize(auctionToken) {
+        emit LockingContractCreated(lockedTokenContract, auctionToken);
+    } catch Error(string memory reason) {
+        revert(string.concat("Initialization failed: ", reason));
+    } catch {
+        revert("Initialization failed");
+    }
-    emit LockingContractCreated(lockedTokenContract, auctionToken);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
lockedTokenContract = TimelockedToken(Clones.clone(TIMELOCKED_TOKEN_IMPLEMENTATION));
lockedTokenContract.initialize(auctionToken);
emit LockingContractCreated(lockedTokenContract, auctionToken);
}
lockedTokenContract = TimelockedToken(Clones.clone(TIMELOCKED_TOKEN_IMPLEMENTATION));
try lockedTokenContract.initialize(auctionToken) {
emit LockingContractCreated(lockedTokenContract, auctionToken);
} catch Error(string memory reason) {
revert(string.concat("Initialization failed: ", reason));
} catch {
revert("Initialization failed");
}
}

Expand Down
Loading
Loading