Merged
Conversation
ororopickpocket
requested changes
May 18, 2021
ororopickpocket
requested changes
May 20, 2021
solidity/contracts/converter/types/liquidity-pool-v1/LiquidityPoolV1Converter.sol
Outdated
Show resolved
Hide resolved
Comment on lines
138
to
161
| const contractRegistry = await web3Func(deploy, "contractRegistry", "ContractRegistry", []); | ||
| const converterFactory = await web3Func(deploy, "converterFactory", "ConverterFactory", []); | ||
| const sovrynSwapFormula = await web3Func(deploy, "sovrynSwapFormula", "SovrynSwapFormula", []); | ||
| const sovrynSwapNetwork = await web3Func(deploy, "sovrynSwapNetwork", "SovrynSwapNetwork", [contractRegistry._address]); | ||
| const converterUpgrader = await web3Func(deploy, "converterUpgrader", "ConverterUpgrader", [contractRegistry._address, addresses.ETH]); | ||
| const converterRegistry = await web3Func(deploy, "converterRegistry", "ConverterRegistry", [contractRegistry._address]); | ||
| const liquidityPoolV1ConverterFactory = await web3Func(deploy, "liquidityPoolV1ConverterFactory", "LiquidityPoolV1ConverterFactory", []); | ||
| const smartToken = await web3Func(deploy, "smartToken", "SmartToken", ["TOKEN", "TKN", 1]); | ||
| const oracle = await web3Func(deploy, "oracle", "Oracle", []); | ||
| const liquidityPoolV1Converter = await web3Func(deploy, "liquidityPoolV1Converter", "LiquidityPoolV1Converter", [smartToken._address, contractRegistry._address, MAX_CONVERSION_FEE]); | ||
|
|
||
| //init sovryn swap formula | ||
| await execute(sovrynSwapFormula.methods.init()); | ||
|
|
||
| // initialize contract registry | ||
| await execute(contractRegistry.methods.registerAddress(Web3.utils.asciiToHex("ContractRegistry"), contractRegistry._address)); | ||
| await execute(contractRegistry.methods.registerAddress(Web3.utils.asciiToHex("ConverterFactory"), converterFactory._address)); | ||
| await execute(contractRegistry.methods.registerAddress(Web3.utils.asciiToHex("SovrynSwapFormula"), sovrynSwapFormula._address)); | ||
| await execute(contractRegistry.methods.registerAddress(Web3.utils.asciiToHex("SovrynSwapNetwork"), sovrynSwapNetwork._address)); | ||
| await execute(contractRegistry.methods.registerAddress(Web3.utils.asciiToHex("SovrynSwapConverterUpgrader"), converterUpgrader._address)); | ||
| await execute(contractRegistry.methods.registerAddress(Web3.utils.asciiToHex("SovrynSwapConverterRegistry"), converterRegistry._address)); | ||
|
|
||
| // initialize converter factory | ||
| await execute(converterFactory.methods.registerTypedConverterFactory(liquidityPoolV1ConverterFactory._address)); |
Contributor
Author
There was a problem hiding this comment.
Do you mean this script not needed and I should add the oracle deployment to the test_deployment_rsk.js script?
Contributor
There was a problem hiding this comment.
i mean why would you register all of these addresses again although they already have been registered on deployment?
Contributor
Author
There was a problem hiding this comment.
Oh okay. Adding oracle deployment to addConverter.js so this won't be needed.
solidity/contracts/converter/types/liquidity-pool-v1/LiquidityPoolV1Converter.sol
Outdated
Show resolved
Hide resolved
ororopickpocket
approved these changes
May 27, 2021
solidity/contracts/converter/types/liquidity-pool-v1/LiquidityPoolV1Converter.sol
Show resolved
Hide resolved
Contributor
|
approved with one comment. |
Contributor
|
would also be nice if
(the script is in an unmerged PR. we can't merge the whole PR, but you can copy the script) |
korepkorep
reviewed
Jun 10, 2021
korepkorep
left a comment
korepkorep
left a comment
There was a problem hiding this comment.
Several code quality issues that don't endanger contracts` security
solidity/contracts/converter/types/liquidity-pool-v1/LiquidityPoolV1Converter.sol
Outdated
Show resolved
Hide resolved
solidity/contracts/converter/types/liquidity-pool-v1/LiquidityPoolV1ConverterMultiAsset.sol
Outdated
Show resolved
Hide resolved
|
|
||
| // ensure that the trade won't deplete the reserve balance | ||
| uint256 targetReserveBalance = reserveBalance(_targetToken); | ||
| assert(amount < targetReserveBalance); |
There was a problem hiding this comment.
Using of assert in production source code is not the best practice. Same for other assert cases
|
|
||
| uint256[] memory reserveMinReturnAmounts = new uint256[](reserveTokens.length); | ||
| for (uint256 i = 0; i < reserveMinReturnAmounts.length; i++) | ||
| reserveMinReturnAmounts[i] = 1; |
solidity/contracts/converter/types/liquidity-pool-v1/LiquidityPoolV1ConverterMultiAsset.sol
Show resolved
Hide resolved
| // transfer funds from the caller in the reserve token | ||
| if (reserveToken == ETH_RESERVE_ADDRESS) { | ||
| if (msg.value > reserveAmount) { | ||
| msg.sender.transfer(msg.value - reserveAmount); |
There was a problem hiding this comment.
Some contracts may revert ether transfers due to lack of gas. Same for other .transfer cases
eMarchenko
approved these changes
Jun 14, 2021
eMarchenko
left a comment
eMarchenko
left a comment
There was a problem hiding this comment.
@korepkorep has performed the review, no security issues have been discovered
…ship Multisig Pool Ownership
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.