Base version #1
Merged
Base version #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FedokDL
reviewed
Apr 10, 2024
contracts/L1/CorePropertiesL1.sol
Outdated
Comment on lines
8
to
12
| address public arbitrumGateway; | ||
| address public treasuryAddress; | ||
|
|
||
| address public lZEnpointAddress; // L1 | ||
| uint256 public destinationChainId; // arbitrum |
There was a problem hiding this comment.
Better to move it to the Factory
contracts/L1/CorePropertiesL1.sol
Outdated
| } | ||
|
|
||
| function setFee(address token_, uint256 fee_) external onlyOwner { | ||
| _fees[token_] = fee_; |
There was a problem hiding this comment.
Check that fee less then 100%
contracts/L1/CorePropertiesL1.sol
Outdated
| _fees[token_] = fee_; | ||
| } | ||
|
|
||
| function setTreasuryAddress(address treasuryAddress_) external onlyOwner { |
There was a problem hiding this comment.
setTreasury, remove Address at the end
contracts/L1/CorePropertiesL1.sol
Outdated
Comment on lines
39
to
47
| function setDeployParams( | ||
| address arbitrumGateway_, | ||
| address lZEnpointAddress_, | ||
| uint256 destinationChainId_ | ||
| ) external onlyOwner { | ||
| arbitrumGateway = arbitrumGateway_; | ||
| lZEnpointAddress = lZEnpointAddress_; | ||
| destinationChainId = destinationChainId_; | ||
| } |
FedokDL
reviewed
Apr 19, 2024
contracts/L1/Distribution.sol
Outdated
| Pool storage pool = pools[poolId_]; | ||
| require(pool.isPublic == pool_.isPublic, "DS: invalid pool type"); | ||
| if (pool_.payoutStart > block.timestamp) { | ||
| require(pool.payoutStart != pool_.payoutStart, "DS: invalid payout start value"); |
There was a problem hiding this comment.
pool.payoutStart == pool_.payoutStart
contracts/L1/Distribution.sol
Outdated
| @@ -327,6 +335,12 @@ contract Distribution is IDistribution, OwnableUpgradeable, UUPSUpgradeable { | |||
| uint256 overplus_ = overplus(); | |||
| require(overplus_ > 0, "DS: overplus is zero"); | |||
|
|
|||
| (uint256 feePercent_, address treasuryAddress_) = coreProperties.getFeeAndTreasury(address(this)); | |||
| uint256 fee_ = _feeAmount(overplus_, feePercent_); | |||
There was a problem hiding this comment.
uint256 fee_ = (amount_ * feePercent_) / PRECISION
contracts/L1/Distribution.sol
Outdated
| uint256 public totalDepositedInPublicPools; | ||
|
|
||
| CorePropertiesL1 public coreProperties; |
There was a problem hiding this comment.
address public feeProperties;
Add it to initialize
contracts/L1/CorePropertiesL1.sol
Outdated
| address public lZEnpointAddress; // L1 | ||
| uint256 public destinationChainId; // arbitrum | ||
|
|
||
| mapping(address => uint256) private _fees; |
contracts/L1/CorePropertiesL1.sol
Outdated
| destinationChainId = destinationChainId_; | ||
| } | ||
|
|
||
| function getFeeAndTreasury(address distributionAddress_) external view returns (uint256, address) { |
There was a problem hiding this comment.
- Check custom fee.
- If not exist, return base fee
contracts/L1/BaseFactoryL1.sol
Outdated
Comment on lines
29
to
31
| address zroPaymentAddress; | ||
| uint16 l2EndpointId; | ||
| bytes adapterParams; |
contracts/L1/BaseFactoryL1.sol
Outdated
|
|
||
| abstract contract BaseFactoryL1 is BaseFactory { | ||
| using EnumerableSet for EnumerableSet.AddressSet; | ||
|
|
There was a problem hiding this comment.
mapping(deployer=>bytes name=>DeployedAddresses)
DeployedAddresses {
distribution: 0x
l1Sender: 0x
}
|
Noticed a typo throughout the codebase - Occurs in at least 8 places so won't list them all, Ctrl + F should do it! |
tster
reviewed
Apr 22, 2024
contracts/L2/Mor20FactoryL2.sol
Outdated
| coreProperties = CorePropertiesL2(coreProperties_); | ||
| } | ||
|
|
||
| function deployMor20OnL1(Mor20DeployParams calldata parameters_) external onlyOwner { |
There was a problem hiding this comment.
shouldn't this method be called deployMor20OnL2?
dovgopoly
reviewed
May 14, 2024
| * @param poolType_ the type of the pool. | ||
| * @param implementation_ the implementation the pool will point to. | ||
| */ | ||
| function setImplementation(uint8 poolType_, address implementation_) public onlyOwner { |
There was a problem hiding this comment.
Should implementation_ be a beacon?
There was a problem hiding this comment.
We decided to do it with ERC1967Proxy to have the chance to remove the upgradability
contracts/L2/L2Factory.sol
Outdated
|
|
||
| IL2MessageReceiver(l2MessageReceiver).L2MessageReceiver__init( | ||
| mor20, | ||
| IL2MessageReceiver.Config(lzExternalDeps.endpoint, address(0), lzExternalDeps.senderChainId) |
There was a problem hiding this comment.
Can we calculate L1Sender in advance?
contracts/L1/L1Factory.sol
Outdated
| lzExternalDeps.adapterParams | ||
| ); | ||
|
|
||
| IL1Sender.DepositTokenConfig memory arbConfig = IL1Sender.DepositTokenConfig( |
There was a problem hiding this comment.
Suggested change
| IL1Sender.DepositTokenConfig memory arbConfig = IL1Sender.DepositTokenConfig( | |
| IL1Sender.DepositTokenConfig memory arbConfig_ = IL1Sender.DepositTokenConfig( |
|
|
||
| import {IFactory} from "./interfaces/IFactory.sol"; | ||
|
|
||
| abstract contract Factory is IFactory, OwnableUpgradeable, PausableUpgradeable, UUPSUpgradeable { |
There was a problem hiding this comment.
Let's add method to precalculate address. It should take deployer as a parameter.
dovgopoly
approved these changes
May 14, 2024
dovgopoly
added a commit
that referenced
this pull request
May 20, 2024
* initial version * Base version (#1) * Base version * Added BaseFactory * Refactored code * Set CoreProperties upgradeable * fixes * refactor logic * updated * removed scripts * Updated package.json * fixes * added more tests * added config * changed swap params * changed package-lock.json * changed package-lock.json * changed ci * changed package-lock.json --------- Co-authored-by: Oleksandr Fedorenko <[email protected]> * added beacon & refactored (#4) * added beacon & refactored * fix event * typo * context * upd enum * tiny adjustments & cov 100% * factories dir & isUpgradeable for all pools * fixed lint * factory ifaces folder --------- Co-authored-by: Ruslan Kasheparov <[email protected]> Co-authored-by: Ruslan Kasheparov <[email protected]> Co-authored-by: Oleksandr Fedorenko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.