-
Notifications
You must be signed in to change notification settings - Fork 31
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
Redeem and Unstake after RevertStake #753
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good to me. My comments are small and largely track the ones I made in #747.
I see there are two different approaches taken in these test files: (a) it
s for each major step in the process and (b) one it
for the whole integrated flow. I prefer the integrated flow approach here, but I nevertheless note the inconsistency and think that a ticket to bring the test files in alignment would be 🌟(cc: @deepesh-kn).
Also: as discussed offline, this PR and #747 both introduce a file (and class), revert_redeem_assertion.js
that are not identical—these changes should be aligned.
test_integration/03_redeem_and_unstake/03_redeem_and_unstake_after_revert.js
Outdated
Show resolved
Hide resolved
test_integration/03_redeem_and_unstake/03_redeem_and_unstake_after_revert.js
Outdated
Show resolved
Hide resolved
test_integration/03_redeem_and_unstake/03_redeem_and_unstake_after_revert.js
Outdated
Show resolved
Hide resolved
test_integration/03_redeem_and_unstake/03_redeem_and_unstake_after_revert.js
Outdated
Show resolved
Hide resolved
test_integration/03_redeem_and_unstake/03_redeem_and_unstake_after_revert.js
Outdated
Show resolved
Hide resolved
test_integration/03_redeem_and_unstake/03_redeem_and_unstake_after_revert.js
Outdated
Show resolved
Hide resolved
Working on the review feedback ⏩ |
- Renamed the test file - Use cogateway.penalty function
Ready for review again 🚀 |
Resolving conflicts ⏰ 😿 |
…edeem_declared # Conflicts: # test_integration/03_redeem_and_unstake/utils/revert_redeem_assertion.js
Conflicts resolved ⛑ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎢
Fixes #738