Create Force Unlock Messages #2733
Conversation
github-actions
bot
added
C:app-wiring
github-actions
bot
removed
the
C:x/gamm
| @@ -71,3 +72,15 @@ message MsgExtendLockup { | |||
| } | |||
|
|
|||
| message MsgExtendLockupResponse { bool success = 1; } | |||
|
|
|||
| message MsgForceUnlock { | |||
| string owner = 1 [ (gogoproto.moretags) = "yaml:\"owner\"" ]; | |||
There was a problem hiding this comment.
Is this an SDK address? If so, shouldn't we add (cosmos_proto.scalar) = "cosmos.AddressString"? Maybe our proto tooling doesn't support this yet?
There was a problem hiding this comment.
Also, option (cosmos.msg.v1.signer) = "from_address";
There was a problem hiding this comment.
what does (cosmos_proto.scalar) = "cosmos.AddressString" and option (cosmos.msg.v1.signer) = "from_address"; do?
Orignial intention was to recieve it as string and then convert it to address
| return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error()) | ||
| } | ||
|
|
||
| return &types.MsgForceUnlockResponse{Success: true}, nil |
There was a problem hiding this comment.
When would we ever return false? Is it worth having the Success field? Are there other fields we might want?
There was a problem hiding this comment.
would return false on the response whenever the transaction fails due to error in msg server, I think this one field would suffice for the field of this response!
| func (k Keeper) GetForceUnlockAllowedAddresses(ctx sdk.Context) (forceUnlockAllowedAddresses []string) { | ||
| return k.GetParams(ctx).ForceUnlockAllowedAddresses | ||
| } | ||
|
|
There was a problem hiding this comment.
non-blocking nit: I'm generally unsure if this method is needed if we can get the addresses from params
There was a problem hiding this comment.
I originally thought this looked messy(having to call params to call get the addresses), but on the second thought I tink that way makes everything more intuitive, reverting this part so that we call from params directly
Co-authored-by: Dev Ojha <[email protected]>
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
|
Hi. Wondering what the status of this issue is? Mainly curious if the API is finalized so that we (Apollo DAO) can implement this in our vault contract. |
Closes: #2727
What is the purpose of the change
This PR creates a msg for force unlocking for the addresses that has been passed via governance and registered as params in the lockup keeper.
This PR also includes adding additional logic to the lockup keeper since we did not support partial unlockings for locks that are already unlocking. ForceUnlock allows force unlocking of locks that are currently being unlocked.
Brief Changelog
Testing and Verifying
Testing WIP: Planning to do PR it separately as test cases are getting big, would prefer separate review for the test cases / edge cases dealt within the test cases
Some of the tests being worked on:
Documentation and Release Note
Unreleasedsection inCHANGELOG.md? (yes / no)x/<module>/spec/) / Osmosis docs repo / not documented)