Create Message for Staking Conversion #5949

mattverse · 2023-08-02T10:43:48Z

Closes: #XXX

What is the purpose of the change

Closes: #5898
WIP

Testing and Verifying

(example:)

Added unit test that validates ...
Added integration tests for end-to-end deployment with ...
Extended integration test for ...
Manually verified the change by ...

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes?
Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

Specification (x/{module}/README.md)
Osmosis documentation site
Code comments?
N/A

proto/osmosis/superfluid/tx.proto

x/superfluid/keeper/stake.go

x/superfluid/keeper/stake_test.go

x/superfluid/keeper/stake.go

ValarDragon · 2023-08-07T15:35:19Z

devbot merge base
fix

# Conflicts: # CHANGELOG.md # go.sum

ValarDragon · 2023-08-07T15:41:16Z

ah sorry, didn't make dev-bot have goimports ignore generated files, now fixed. (Though TBH that is a bit odd that our codegen doesn't obey goimports)

app/keepers/keepers.go

proto/osmosis/superfluid/tx.proto

x/superfluid/keeper/migrate.go

x/superfluid/types/msgs.go

ValarDragon

Requested changes for relaxing valset delegation

.golangci.yml

proto/osmosis/superfluid/tx.proto

x/superfluid/keeper/migrate.go

x/superfluid/keeper/migrate_test.go

x/superfluid/keeper/stake.go

proto/osmosis/superfluid/tx.proto

x/superfluid/keeper/migrate.go

x/superfluid/keeper/stake.go

x/superfluid/keeper/stake_test.go

mattverse · 2023-08-11T17:50:09Z

Merge conflict situation is pretty bad here 😭

mattverse · 2023-08-11T18:01:22Z

yes!!!

czarcas7ic · 2023-08-11T19:48:13Z

Just did a quick pass and it appears the merge conflicts were fixed, nice job Matt. Will now do another review, should be my final one, thanks again Matt :)

czarcas7ic

Awesome job with all the iterations on this!! Must have been really exhausting :(

I think we should be looking for two more approvals, but I don't think they will be very far behind as it looks like we have converged on pretty much all comments.

Again, great job :)

x/superfluid/keeper/stake.go

x/superfluid/keeper/stake_test.go

mattverse · 2023-08-12T12:53:34Z

Awesome job with all the iterations on this!! Must have been really exhausting :(

I think we should be looking for two more approvals, but I don't think they will be very far behind as it looks like we have converged on pretty much all comments.

Again, great job :)

Thanks so much! Thanks for the detailed review iterations as well :) 🙏

p0mvn

Phenomenal work on addressing multiple rounds of feedback and testing everything thoroughly, including localosmosis!

The summaries of localosmosis tests were especially helpful.

I would like to suggest for the future to try adding comment test summaries about the invariants being tested by each test. This would make the test review easier and faster

x/superfluid/keeper/stake.go

x/superfluid/keeper/stake_test.go

Co-authored-by: Roman <[email protected]>

AlpinYukseloglu

LGTM, nice work. No major issues came up on this pass through

AlpinYukseloglu · 2023-08-14T18:46:12Z

x/superfluid/keeper/stake.go

+	originalBondDenomAmt := exitCoins.AmountOf(bondDenom)
+
+	// track how much non-uosmo tokens we have converted to uosmo
+	totalAmtCoverted = sdk.ZeroInt()


nit: coverted -> converted

AlpinYukseloglu · 2023-08-14T18:46:44Z

x/superfluid/keeper/stake.go

+// - If valAddr not provided and valset exists, valsetpref.Delegate
+// - If valAddr not provided and valset delegation is not possible, refer back to original lock's superfluid validator if it was a superfluid lock
+// - Else: error
+func (k Keeper) delegateBaseOnValsetPref(ctx sdk.Context, sender sdk.AccAddress, valAddr, originalSuperfluidValAddr string, totalAmtToStake sdk.Int) error {


nit: delegateBaseOnValsetPref -> delegateBasedOnValsetPref

requested dismissal

* WIP * Almost done, need more test cases... * Finish adding test cases * Use val set * Add changelog * Fix simple problems * Update tx.pb.go * Remove partial unlocking * Use val set delegation and edit test cases * Update proto/osmosis/superfluid/tx.proto Co-authored-by: Adam Tucker <[email protected]> * Update x/superfluid/keeper/stake.go Co-authored-by: alpo <[email protected]> * Romans comment, add balancer check * Adam's comment * Change errors from bool to check error * Add validate basic, bring back partial share migration for liquid gamm shares * Try abstracting check logics from test * Romans comment * Add cli * Fix cli * Update x/superfluid/keeper/stake.go * Update x/superfluid/client/cli/tx.go * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Roman and Adam * Update x/superfluid/keeper/stake.go * Adams comment * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Romans comments --------- Co-authored-by: devbot-wizard <[email protected]> Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: alpo <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Roman <[email protected]> (cherry picked from commit d99138f)

* WIP * Almost done, need more test cases... * Finish adding test cases * Use val set * Add changelog * Fix simple problems * Update tx.pb.go * Remove partial unlocking * Use val set delegation and edit test cases * Update proto/osmosis/superfluid/tx.proto Co-authored-by: Adam Tucker <[email protected]> * Update x/superfluid/keeper/stake.go Co-authored-by: alpo <[email protected]> * Romans comment, add balancer check * Adam's comment * Change errors from bool to check error * Add validate basic, bring back partial share migration for liquid gamm shares * Try abstracting check logics from test * Romans comment * Add cli * Fix cli * Update x/superfluid/keeper/stake.go * Update x/superfluid/client/cli/tx.go * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Roman and Adam * Update x/superfluid/keeper/stake.go * Adams comment * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Romans comments --------- Co-authored-by: devbot-wizard <[email protected]> Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: alpo <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Roman <[email protected]> (cherry picked from commit d99138f) Co-authored-by: Matt, Park <[email protected]>

…bs#6094) * WIP * Almost done, need more test cases... * Finish adding test cases * Use val set * Add changelog * Fix simple problems * Update tx.pb.go * Remove partial unlocking * Use val set delegation and edit test cases * Update proto/osmosis/superfluid/tx.proto Co-authored-by: Adam Tucker <[email protected]> * Update x/superfluid/keeper/stake.go Co-authored-by: alpo <[email protected]> * Romans comment, add balancer check * Adam's comment * Change errors from bool to check error * Add validate basic, bring back partial share migration for liquid gamm shares * Try abstracting check logics from test * Romans comment * Add cli * Fix cli * Update x/superfluid/keeper/stake.go * Update x/superfluid/client/cli/tx.go * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Roman and Adam * Update x/superfluid/keeper/stake.go * Adams comment * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Romans comments --------- Co-authored-by: devbot-wizard <[email protected]> Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: alpo <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Roman <[email protected]> (cherry picked from commit d99138f) Co-authored-by: Matt, Park <[email protected]>

* WIP * Almost done, need more test cases... * Finish adding test cases * Use val set * Add changelog * Fix simple problems * Update tx.pb.go * Remove partial unlocking * Use val set delegation and edit test cases * Update proto/osmosis/superfluid/tx.proto Co-authored-by: Adam Tucker <[email protected]> * Update x/superfluid/keeper/stake.go Co-authored-by: alpo <[email protected]> * Romans comment, add balancer check * Adam's comment * Change errors from bool to check error * Add validate basic, bring back partial share migration for liquid gamm shares * Try abstracting check logics from test * Romans comment * Add cli * Fix cli * Update x/superfluid/keeper/stake.go * Update x/superfluid/client/cli/tx.go * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Roman and Adam * Update x/superfluid/keeper/stake.go * Adams comment * Update x/superfluid/keeper/stake.go Co-authored-by: Roman <[email protected]> * Romans comments --------- Co-authored-by: devbot-wizard <[email protected]> Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: alpo <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Roman <[email protected]>

WIP

5f48cb3

mattverse requested review from stackman27 and AlpinYukseloglu as code owners August 2, 2023 10:43

mattverse marked this pull request as draft August 2, 2023 10:43

github-actions bot added the C:x/superfluid label Aug 2, 2023

sunnya97 reviewed Aug 2, 2023

View reviewed changes

proto/osmosis/superfluid/tx.proto Show resolved Hide resolved

sunnya97 reviewed Aug 2, 2023

View reviewed changes

proto/osmosis/superfluid/tx.proto Show resolved Hide resolved

mattverse commented Aug 2, 2023

View reviewed changes

x/superfluid/keeper/stake.go Outdated Show resolved Hide resolved

Almost done, need more test cases...

2526268

github-actions bot added T:CI C:app-wiring Changes to the app folder labels Aug 3, 2023

ValarDragon added the V:state/breaking State machine breaking PR label Aug 3, 2023

mattverse added 2 commits August 4, 2023 22:17

Finish adding test cases

9f10685

Use val set

8516a9b

mattverse marked this pull request as ready for review August 4, 2023 14:16

mattverse commented Aug 4, 2023

View reviewed changes

x/superfluid/keeper/stake_test.go Outdated Show resolved Hide resolved

Add changelog

4986e0e

mattverse commented Aug 7, 2023

View reviewed changes

x/superfluid/keeper/stake.go Outdated Show resolved Hide resolved

devbot-wizard added 2 commits August 7, 2023 15:35

Merge branch 'main' into mattverse/stake-multi0msg

18e8898

# Conflicts: # CHANGELOG.md # go.sum

Fix simple problems

6000313

Update tx.pb.go

343be73

ValarDragon reviewed Aug 7, 2023

View reviewed changes

app/keepers/keepers.go Show resolved Hide resolved

ValarDragon reviewed Aug 7, 2023

View reviewed changes

proto/osmosis/superfluid/tx.proto Outdated Show resolved Hide resolved

ValarDragon reviewed Aug 7, 2023

View reviewed changes

x/superfluid/keeper/migrate.go Outdated Show resolved Hide resolved

ValarDragon reviewed Aug 7, 2023

View reviewed changes

x/superfluid/types/msgs.go Show resolved Hide resolved

ValarDragon previously requested changes Aug 7, 2023

View reviewed changes

AlpinYukseloglu reviewed Aug 7, 2023

View reviewed changes

czarcas7ic reviewed Aug 7, 2023

View reviewed changes

mattverse force-pushed the mattverse/stake-multi0msg branch from eab4bfa to 18f4bd7 Compare August 11, 2023 17:47

Fix merge conflict

d7e9511

czarcas7ic approved these changes Aug 11, 2023

View reviewed changes

x/superfluid/keeper/stake.go Outdated Show resolved Hide resolved

x/superfluid/keeper/stake_test.go Show resolved Hide resolved

czarcas7ic and others added 3 commits August 11, 2023 16:50

Update x/superfluid/keeper/stake.go

e5a054c

fix merge conflict

33ac505

Adams comment

fe4b64c

p0mvn approved these changes Aug 14, 2023

View reviewed changes

x/superfluid/keeper/stake.go Outdated Show resolved Hide resolved

x/superfluid/keeper/stake_test.go Outdated Show resolved Hide resolved

x/superfluid/keeper/stake_test.go Show resolved Hide resolved

mattverse and others added 2 commits August 14, 2023 22:46

Update x/superfluid/keeper/stake.go

c7b9fdb

Co-authored-by: Roman <[email protected]>

Romans comments

9244686

AlpinYukseloglu approved these changes Aug 14, 2023

View reviewed changes

ValarDragon self-requested a review August 14, 2023 19:17

czarcas7ic merged commit d99138f into main Aug 15, 2023

czarcas7ic deleted the mattverse/stake-multi0msg branch August 15, 2023 17:24

mattverse added the A:backport/v17.x backport patches to v17.x branch label Aug 17, 2023

mergify bot mentioned this pull request Aug 17, 2023

Create Message for Staking Conversion (backport #5949) #6094

Merged

github-actions bot mentioned this pull request Feb 15, 2024

Broken Links Detected #7491

Closed

github-actions bot mentioned this pull request Mar 15, 2024

Broken Links Detected #7752

Closed

github-actions bot mentioned this pull request Apr 15, 2024

Broken Links Detected #8046

Closed

github-actions bot mentioned this pull request Aug 15, 2024

Broken Links Detected notional-labs/osmosis#71

Closed

github-actions bot mentioned this pull request Oct 15, 2024

Broken Links Detected notional-labs/osmosis#75

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Message for Staking Conversion #5949

Create Message for Staking Conversion #5949

mattverse commented Aug 2, 2023 •

edited by czarcas7ic

Loading

ValarDragon commented Aug 7, 2023

ValarDragon commented Aug 7, 2023 •

edited

Loading

ValarDragon left a comment

mattverse commented Aug 11, 2023

mattverse commented Aug 11, 2023

czarcas7ic commented Aug 11, 2023

czarcas7ic left a comment

mattverse commented Aug 12, 2023

p0mvn left a comment

AlpinYukseloglu left a comment

AlpinYukseloglu Aug 14, 2023

AlpinYukseloglu Aug 14, 2023

Create Message for Staking Conversion #5949

Create Message for Staking Conversion #5949

Conversation

mattverse commented Aug 2, 2023 • edited by czarcas7ic Loading

What is the purpose of the change

Testing and Verifying

Documentation and Release Note

ValarDragon commented Aug 7, 2023

ValarDragon commented Aug 7, 2023 • edited Loading

ValarDragon left a comment

Choose a reason for hiding this comment

mattverse commented Aug 11, 2023

mattverse commented Aug 11, 2023

czarcas7ic commented Aug 11, 2023

czarcas7ic left a comment

Choose a reason for hiding this comment

mattverse commented Aug 12, 2023

p0mvn left a comment

Choose a reason for hiding this comment

AlpinYukseloglu left a comment

Choose a reason for hiding this comment

AlpinYukseloglu Aug 14, 2023

Choose a reason for hiding this comment

AlpinYukseloglu Aug 14, 2023

Choose a reason for hiding this comment

mattverse commented Aug 2, 2023 •

edited by czarcas7ic

Loading

ValarDragon commented Aug 7, 2023 •

edited

Loading