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

SOV-3497 move sovrynSwapsImpl to internal protocol module #530

Merged
merged 14 commits into from
Dec 18, 2023

Conversation

cwsnt
Copy link
Contributor

@cwsnt cwsnt commented Nov 20, 2023

No description provided.

add unit test for new module
@cwsnt cwsnt marked this pull request as ready for review November 22, 2023 04:16
@cwsnt cwsnt requested a review from tjcloa November 22, 2023 04:16
@tjcloa tjcloa requested a review from night-pm November 25, 2023 15:44
contracts/swaps/connectors/SwapsImplSovrynSwapInternal.sol Outdated Show resolved Hide resolved
contracts/interfaces/ISovryn.sol Outdated Show resolved Hide resolved
contracts/swaps/connectors/SwapsImplSovrynSwapInternal.sol Outdated Show resolved Hide resolved
contracts/swaps/connectors/SwapsImplSovrynSwapInternal.sol Outdated Show resolved Hide resolved
hardhat/tasks/sips/args/sipArgs.js Outdated Show resolved Hide resolved
hardhat/tasks/sips/args/sipArgs.js Show resolved Hide resolved
tests/Utils/initializer.js Show resolved Hide resolved
tests/swaps/SwapsExternal.js Show resolved Hide resolved
tests/swaps/SwapsExternal.js Show resolved Hide resolved
@tjcloa
Copy link
Contributor

tjcloa commented Nov 28, 2023

pls either remove interface ISwapsImpl or update it if there is a way we can use it - remove internalSwap function and rename internalExpectedRate and internalExpectedReturn to swapsImplExpectedRate and swapsImplExpectedReturn respectively

@cwsnt cwsnt requested a review from tjcloa November 30, 2023 01:16
Copy link
Contributor

@tjcloa tjcloa left a comment

Choose a reason for hiding this comment

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

almost there
some minor changes requested

deployment/deploy/2060-deploy-ProtocolModules.js Outdated Show resolved Hide resolved
deployment/deploy/2070-deploy-ReplaceProtocolModules.js Outdated Show resolved Hide resolved
log(col.bgBlue("Protocol modules are deployed"));
log(
col.bgBlue(
"Prepare and run SIP function in sips.js to create the proposal with params:"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Prepare and run SIP function in sips.js to create the proposal with params:"
"Prepare a SIP creation function in sipArgs.js and run hardhat task `sips:create` to create proposal replacing modules:"

and print replacing modules names and their addresses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

Copy link
Contributor

Choose a reason for hiding this comment

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

and print replacing modules names and their addresses

hardhat/tasks/sips/args/sipArgs.js Show resolved Hide resolved
tests-onchain/sipSov3497.test.js Outdated Show resolved Hide resolved
tests-onchain/sipSov3497.test.js Outdated Show resolved Hide resolved
tests-onchain/sipSov3497.test.js Outdated Show resolved Hide resolved
tests-onchain/sipSov3497.test.js Show resolved Hide resolved
hardhat/tasks/sips/args/sipArgs.js Show resolved Hide resolved
@cwsnt cwsnt requested a review from tjcloa November 30, 2023 14:25
Copy link
Contributor

@tjcloa tjcloa left a comment

Choose a reason for hiding this comment

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

approved but with one comment - missing from the previous review

log(col.bgBlue("Protocol modules are deployed"));
log(
col.bgBlue(
"Prepare and run SIP function in sips.js to create the proposal with params:"
Copy link
Contributor

Choose a reason for hiding this comment

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

and print replacing modules names and their addresses

@tjcloa tjcloa changed the base branch from development to SOV-3436-Proton-release November 30, 2023 15:47
Copy link

@night-pm night-pm left a comment

Choose a reason for hiding this comment

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

LGTM

@cwsnt cwsnt merged commit 8cc0c9d into SOV-3436-Proton-release Dec 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants