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.

Sign up for GitHub

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

Jump to bottom

fix(stride): Update Redeem method, types and tests. #1935

Merged
merged 15 commits into from Oct 25, 2023
Merged

fix(stride): Update Redeem method, types and tests. #1935

merged 15 commits into from Oct 25, 2023

Conversation

Vvaradinov
Copy link
Contributor

Description

Based on the information shared by the Stride team this PR updates the Redeem method with an additional evmosAddress where the unstaked tokens will be received. Furthermore, this PR modifies the memo creating function, types, events and tests.

Closes #XXX

@Vvaradinov Vvaradinov requested a review from a team as a code owner October 25, 2023 07:59
@Vvaradinov Vvaradinov requested review from MalteHerrmann, GAtom22, fedekunze and 0xstepit and removed request for a team October 25, 2023 07:59
fedekunze
fedekunze requested changes Oct 25, 2023
View reviewed changes
precompiles/outposts/stride/IStrideOutpost.sol Outdated Show resolved Hide resolved
precompiles/outposts/stride/events.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/events.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/events.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/IStrideOutpost.sol Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 25, 2023 •
edited

Codecov Report

Merging #1935 (ad731da) into main (51c9612) will decrease coverage by 0.13%.
The diff coverage is 19.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1935      +/-   ##
==========================================
- Coverage   69.67%   69.54%   -0.13%     
==========================================
  Files         324      324              
  Lines       23964    24017      +53     
==========================================
+ Hits        16697    16703       +6     
- Misses       6436     6478      +42     
- Partials      831      836       +5
Files Coverage Δ
precompiles/outposts/stride/events.go 68.18% <66.66%> (-2.79%) ⬇️
precompiles/outposts/stride/stride.go 30.30% <0.00%> (ø)
precompiles/outposts/stride/tx.go 0.00% <0.00%> (ø)
precompiles/outposts/stride/types.go 20.21% <14.28%> (-10.90%) ⬇️

0xstepit
0xstepit reviewed Oct 25, 2023
View reviewed changes
Copy link
Contributor

@0xstepit 0xstepit left a comment

Choose a reason for hiding this comment

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

Well done @Vvaradinov! Left some comment

precompiles/outposts/stride/IStrideOutpost.sol Outdated Show resolved Hide resolved
precompiles/outposts/stride/IStrideOutpost.sol Outdated Show resolved Hide resolved
precompiles/outposts/stride/tx.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/types_test.go Show resolved Hide resolved
MalteHerrmann
MalteHerrmann approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@MalteHerrmann MalteHerrmann left a comment

Choose a reason for hiding this comment

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

Looking good! Left some small comments 🙏

precompiles/outposts/stride/IStrideOutpost.sol Outdated Show resolved Hide resolved
precompiles/outposts/stride/IStrideOutpost.sol Outdated Show resolved Hide resolved
precompiles/outposts/stride/types.go Show resolved Hide resolved
fedekunze
fedekunze approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM

precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
Vvaradinov and others added 2 commits October 25, 2023 12:48
@Vvaradinov
fix: add ValidateBasic for CreateMemo struct
c62bebb
@Vvaradinov @fedekunze
Apply suggestions from code review
a41cbbd 
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
fedekunze
fedekunze reviewed Oct 25, 2023
View reviewed changes
precompiles/outposts/stride/types.go Show resolved Hide resolved
precompiles/outposts/stride/types.go Show resolved Hide resolved
precompiles/outposts/stride/types.go Outdated Show resolved Hide resolved
@fedekunze
Copy link
Contributor

@Vvaradinov tests are failing

--- FAIL: TestCreateMemo (0.00s)
    --- FAIL: TestCreateMemo/success_-_redeem_stake (0.00s)
        types_test.go:45: 
            	Error Trace:	/home/runner/work/evmos/evmos/precompiles/outposts/stride/types_test.go:45
            	Error:      	Received unexpected error:
            	            	IBCReceiver cannot be empty when action is RedeemStake
            	Test:       	TestCreateMemo/success_-_redeem_stake
            	Messages:   	expected no error while creating memo

@Vvaradinov Vvaradinov merged commit a051e9b into main Oct 25, 2023
33 of 36 checks passed
@Vvaradinov Vvaradinov deleted the Vvaradinov/stride-update-redeem branch October 25, 2023 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fedekunze fedekunze fedekunze approved these changes

@MalteHerrmann MalteHerrmann MalteHerrmann approved these changes

@0xstepit 0xstepit 0xstepit approved these changes

@GAtom22 GAtom22 Awaiting requested review from GAtom22 GAtom22 is a code owner automatically assigned from evmos/core-engineering

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Vvaradinov @fedekunze @MalteHerrmann @0xstepit