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

[LUM-570] - Add msg to edit a deposit #32

Merged
merged 8 commits into from
Jun 29, 2023
Merged

[LUM-570] - Add msg to edit a deposit #32

merged 8 commits into from
Jun 29, 2023

Conversation

Ricardo-Remy
Copy link
Contributor

@Ricardo-Remy Ricardo-Remy commented Jun 2, 2023 •
edited
Loading

Introduction

This PR enables a deposit edition in order to avoid a withdrawal+redeposit situation when someone wants to change the following deposit params:

  • Edit winner address
  • Edit sponsor mode

Changelog

  • Add new MsgServer DepositEdit which allows to change params winner_address and sponsor
  • Based on cosmos sdk 0.47
  • Unit tests for keeper and msgServer

Testing

Testing the EditDeposit is planned on the public Lum Network testnet before being released for the next upgrade

@Ricardo-Remy Ricardo-Remy changed the title Feature/lum 570 rc/v1.5 [LUM-570] - Add msg to edit a deposit Jun 2, 2023
@Ricardo-Remy Ricardo-Remy changed the base branch from rc/v1.5 to feature/lum-710 June 5, 2023 15:13
lebascou
lebascou requested changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@lebascou lebascou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM - requesting minor changes
Also please rebase on latest changes from LUM-710 just in case.

x/millions/types/message_deposit_edit.go Show resolved Hide resolved
x/millions/keeper/keeper_deposit.go Show resolved Hide resolved
@lebascou lebascou added ready for review ⌛ PR ready for peer review DO NOT MERGE ⏲️ Do not merge unless decided otherwise labels Jun 9, 2023
@lebascou
Copy link
Contributor

lebascou commented Jun 9, 2023

Adding the label DO NOT MERGE until we finalise the review of PR#28 lum-710 in an attempt to reduce its review complexity.

lebascou
lebascou approved these changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@lebascou lebascou left a comment

Choose a reason for hiding this comment

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

LGTM

@lebascou lebascou added ready to merge 👌 PR ready to be merged and removed ready for review ⌛ PR ready for peer review labels Jun 9, 2023
@lebascou lebascou removed the DO NOT MERGE ⏲️ Do not merge unless decided otherwise label Jun 29, 2023
Base automatically changed from feature/lum-710 to rc/v1.5 June 29, 2023 13:31
@lebascou lebascou merged commit 865f646 into rc/v1.5 Jun 29, 2023
@lebascou lebascou deleted the feature/LUM-570-rc/v1.5 branch June 29, 2023 13:51
@lebascou lebascou mentioned this pull request Jun 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lebascou lebascou lebascou approved these changes

Assignees
No one assigned
Labels
ready to merge 👌 PR ready to be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ricardo-Remy @lebascou