Autopilot enhancement #771

jstr1121 · 2023-05-12T07:59:22Z

Context and purpose of the change

The goal of this PR is to support two more features on autopilot module

RedeemStake as part of ibc transfer
LiquidStake and IBC transfer back liquid staking token as part of ibc transfer

Note: This PR moved from #626

Brief Changelog

MsgLiquidStake is returning received liquid staking token amount
Add unit test for RedeemStake and LiquidStakeAndIBCTransfer
Add integration test for RedeemStake and LiquidStakeAndIBCTransfer

Author's Checklist

I have...

Run and PASSED locally all GAIA integration tests
If the change is contentful, I either:
- Added a new unit test OR
- Added test cases to existing unit tests
OR this change is a trivial rework / code cleanup without any test coverage

If skipped any of the tests above, explain.

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

reviewed state machine logic
reviewed API design and naming
manually tested (if applicable)
confirmed the author wrote unit tests for new logic
reviewed documentation exists and is accurate

How to test integration test on local

Update DefaultStakeibcActive to true at x/autopilot/types/params.go
Comment out all skip "DefaultActive set to false, skip test" on dockernet/tests/integration_tests.bats
Install docker image for stride make build-docker build=s
Start dockernet with make start-docker
Run integration test with make test-integration-docker
Ensure the result is as following

integration_tests.bats
 ✓ [INTEGRATION-BASIC-GAIA] host zones successfully registered
 ✓ [INTEGRATION-BASIC-GAIA] ibc transfer updates all balances
 ✓ [INTEGRATION-BASIC-GAIA] liquid stake mint and transfer
 ✓ [INTEGRATION-BASIC-GAIA] packet forwarding automatically liquid stakes
 ✓ [INTEGRATION-BASIC-GAIA] transfer stuatom to host chain
 ✓ [INTEGRATION-BASIC-GAIA] packet forwarding automatically liquid stake and ibc transfer stAsset to original network
 ✓ [INTEGRATION-BASIC-GAIA] tokens on GAIA were staked
 ✓ [INTEGRATION-BASIC-GAIA] packet forwarding automatically redeem stake
 ✓ [INTEGRATION-BASIC-GAIA] redemption works
 ✓ [INTEGRATION-BASIC-GAIA] claimed tokens are properly distributed
 ✓ [INTEGRATION-BASIC-GAIA] rewards are being reinvested, exchange rate updating
 ✓ [INTEGRATION-BASIC-GAIA] rewards are being distributed to stakers

12 tests, 0 failures

How to test Hub -> Stride -> Osmosis

Update dockernet/config.sh to HOST_CHAINS=(GAIA OSMO)
Start dockernet with make start-docker build=sgor
Test following script and ensure OSMO_ADDRESS gets stuatom ibc denom

GAIA_ADDRESS=$(./build/gaiad keys show -a gval1 --home=./dockernet/state/gaia1)
STRIDE_ADDRESS=stride1jrmtt5c6z8h5yrrwml488qnm7p3vxrrml2kgvl
OSMO_ADDRESS=osmo1dtq0y9reqst7d99fd3c7x6dflh4eazm4lx5sk9
GAIA_STRIDE_CHAN=channel-0
STRIDE_OSMO_CHAN=channel-1
memo='{ "autopilot": { "receiver": "'$STRIDE_ADDRESS'",  "stakeibc": { "stride_address": "'$STRIDE_ADDRESS'", "action": "LiquidStake", "ibc_receiver": "'$OSMO_ADDRESS'", "transfer_channel": "'$STRIDE_OSMO_CHAN'"  } } }'
./build/gaiad tx ibc-transfer transfer transfer $GAIA_STRIDE_CHAN "$memo" 10000uatom --from gval1 -y --home=./dockernet/state/gaia1
./build/gaiad query tx 31A1480A5F025F900A684F7CFD92F2FE06B4307CA81BD1E01AB31E9C01BFC7DF --home=./dockernet/state/gaia1
./build/gaiad query bank balances $GAIA_ADDRESS --home=./dockernet/state/gaia1
./build/strided query bank balances $STRIDE_ADDRESS --home=./dockernet/state/stride1
./build/osmosisd query bank balances $OSMO_ADDRESS --home=./dockernet/state/osmo1

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes?
Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md?
This pull request updates existing proto field values (and require a backend and frontend migration)?
Does this pull request change existing proto field names (and require a frontend migration)?
How is the feature or change documented?
- not applicable
- jira ticket XXX
- specification (x/<module>/spec/)
- README.md
- not documented

…atible with json formula

…eem and aidrop

fedekunze · 2023-05-25T11:11:33Z

x/autopilot/types/parser.go

@@ -27,8 +27,10 @@ type ModuleRoutingInfo interface {

 // Packet metadata info specific to Stakeibc (e.g. 1-click liquid staking)
 type StakeibcPacketMetadata struct {
-	Action        string `json:"action"`
-	StrideAddress string `json:"stride_address"`
+	Action          string `json:"action"`


why not use an enum here?

fedekunze · 2023-05-25T11:12:22Z

x/autopilot/types/parser.go

+	IbcReceiver     string `json:"ibc_receiver,omitempty"`
+	TransferChannel string `json:"transfer_channel,omitempty"`


if these are empty it's assumed that the token is returned to the original sender over the same source channel?

Correct. These are omitted only when the receiver is original sender through original transfer channel.

fedekunze · 2023-05-25T11:12:59Z

x/autopilot/types/parser.go

+	case "LiquidStake":
+	case "RedeemStake":


these seem that are not set up

It's validation if the action is one of LiquidStake or RedeemStake, more could be added later and just used this format for validation

fedekunze · 2023-05-25T11:14:03Z

x/autopilot/types/parser.go

@@ -27,8 +27,10 @@ type ModuleRoutingInfo interface {

 // Packet metadata info specific to Stakeibc (e.g. 1-click liquid staking)
 type StakeibcPacketMetadata struct {


nit

Suggested change

type StakeibcPacketMetadata struct {

type StakeIBCPacketMetadata struct {

fedekunze · 2023-05-25T11:14:48Z

x/autopilot/module_ibc.go

+		case "RedeemStake":
+			// Try to redeem stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation
+			if err := im.keeper.TryRedeemStake(ctx, packet, newData, routingInfo); err != nil {
+				im.keeper.Logger(ctx).Error(fmt.Sprintf("Error redeem staking packet from autopilot for %s: %s", newData.Sender, err.Error()))


Suggested change

im.keeper.Logger(ctx).Error(fmt.Sprintf("Error redeem staking packet from autopilot for %s: %s", newData.Sender, err.Error()))

im.keeper.Logger(ctx).Debug("failed redeem staking packet from autopilot", "sender" newData.Sender.String(), "error", err.Error())

x/autopilot/module_ibc.go

fedekunze · 2023-05-25T11:19:23Z

x/autopilot/keeper/redeemstake.go

+	voucherPrefix := transfertypes.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())
+	stAssetDenom := newData.Denom[len(voucherPrefix):]


pretty sure there is a util for this

fedekunze · 2023-05-25T11:21:34Z

x/autopilot/keeper/redeemstake.go

+	if !transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), newData.Denom) {
+		return fmt.Errorf("the ibc tokens are not supported for redeem stake")
+	}


what's the redeem stake flow? isn't the receiver chain in this case Stride?

fedekunze · 2023-05-25T11:24:08Z

x/autopilot/keeper/redeemstake.go

+	}
+
+	// Note: newData.denom is ibc denom for st assets - e.g. ibc/xxx
+	var token = sdk.NewCoin(newData.Denom, amount)


double-check if the packet data is validated, otherwise this line panics if the denom or amount are invalid

fedekunze · 2023-05-25T11:28:42Z

x/autopilot/keeper/redeemstake.go

+	return k.RunRedeemStake(ctx, strideAddress, packetMetadata.IbcReceiver, hostZoneDenom, token, []metrics.Label{})
+}
+
+func (k Keeper) RunRedeemStake(ctx sdk.Context, addr sdk.AccAddress, receiver string, hostZoneDenom string, token sdk.Coin, labels []metrics.Label) error {


highly recommend using the spread operator for optional slices

github-actions · 2023-06-09T00:03:48Z

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!

…nhancement

…nit test failure for total escrow amount set on ibc escrow mock

github-actions · 2023-07-12T00:03:58Z

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!

github-actions · 2023-08-16T00:03:31Z

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!

riley-stride

Reviewed "remove stride_address field on functional info" commit 088fb2d. Looks good.

riley-stride · 2023-08-23T20:28:29Z

x/autopilot/types/parser.go

 	IbcReceiver     string `json:"ibc_receiver,omitempty"`
 	TransferChannel string `json:"transfer_channel,omitempty"`
 }

 // Packet metadata info specific to Claim (e.g. airdrops for non-118 coins)
 type ClaimPacketMetadata struct {
-	StrideAddress string
 }

 // Validate stakeibc packet metadata fields
 // including the stride address and action type


nit: rm "stride address and"

riley-stride · 2023-08-23T20:29:00Z

x/autopilot/types/parser.go

@@ -57,10 +51,6 @@ func (m StakeibcPacketMetadata) Validate() error {

 // Validate claim packet metadata includes the stride address
 func (m ClaimPacketMetadata) Validate() error {


Should this fn be removed now that it validates nothing?

github-actions · 2023-09-08T00:03:38Z

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!

asalzmann · 2023-11-28T19:17:50Z

x/autopilot/keeper/liquidstake.go

@@ -38,9 +38,16 @@ func (k Keeper) TryLiquidStaking(
 	}

 	// Note: newData.denom is base denom e.g. uatom - not ibc/xxx
-	var token = sdk.NewCoin(newData.Denom, amount)
+	var token = sdk.Coin{


Not needed afaict

Closes: #771 ## Context and purpose of the change Add `RedeemStake` to autopilot. ## Brief Changelog * Add integration test for `RedeemStake` * Add unittest for `RedeemStake` * Integrate with autopilot module

move autopilot redeem stake, liquid stake and ibc transfer to be comp…

f717a7f

…atible with json formula

github-actions bot added C:app-wiring C:proto C:stakeibc T:build labels May 12, 2023

jstr1121 added 3 commits May 16, 2023 22:30

Update MaxMemoCharLength to 512 and resolve unit test failures on red…

c712c6b

…eem and aidrop

remove deprecated sdkerrors.Wrapf

7f95b74

resolve lint issue for fmt.Sprintf usage

419ef76

fedekunze reviewed May 25, 2023

View reviewed changes

github-actions bot added the Stale label Jun 9, 2023

github-actions bot closed this Jun 15, 2023

jstr1121 reopened this Jun 26, 2023

jstr1121 added 3 commits June 27, 2023 00:50

Merge branch 'main' of github.com:Stride-Labs/stride into autopilot_e…

eaceeff

…nhancement

Resolve stride version and ibc-go version on autopilot PR & Resolve u…

7cb6a93

…nit test failure for total escrow amount set on ibc escrow mock

add validation on coin object to avoid panic on NewCoin function call

e8cf2de

github-actions bot removed the Stale label Jun 27, 2023

remove stride_address field on functional info

088fb2d

github-actions bot added the Stale label Jul 12, 2023

github-actions bot closed this Jul 18, 2023

riley-stride reopened this Jul 31, 2023

github-actions bot removed the Stale label Aug 1, 2023

github-actions bot added the Stale label Aug 16, 2023

github-actions bot closed this Aug 23, 2023

riley-stride reopened this Aug 23, 2023

riley-stride reviewed Aug 23, 2023

View reviewed changes

github-actions bot removed the Stale label Aug 24, 2023

github-actions bot added the Stale label Sep 8, 2023

github-actions bot closed this Sep 15, 2023

asalzmann reviewed Nov 28, 2023

View reviewed changes

This was referenced Nov 28, 2023

Refresh autopilot LS + forward #996

Merged

add RedeemStake to autopilot #1012

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autopilot enhancement #771

Autopilot enhancement #771

jstr1121 commented May 12, 2023 •

edited

Loading

fedekunze May 25, 2023

fedekunze May 25, 2023

jstr1121 Jun 26, 2023

fedekunze May 25, 2023

jstr1121 Jun 26, 2023

fedekunze May 25, 2023

fedekunze May 25, 2023

fedekunze May 25, 2023

fedekunze May 25, 2023

fedekunze May 25, 2023

fedekunze May 25, 2023

github-actions bot commented Jun 9, 2023

github-actions bot commented Jul 12, 2023

github-actions bot commented Aug 16, 2023

riley-stride left a comment

riley-stride Aug 23, 2023

riley-stride Aug 23, 2023

github-actions bot commented Sep 8, 2023

asalzmann Nov 28, 2023

		IbcReceiver string `json:"ibc_receiver,omitempty"`
		TransferChannel string `json:"transfer_channel,omitempty"`

		@@ -27,8 +27,10 @@ type ModuleRoutingInfo interface {

		// Packet metadata info specific to Stakeibc (e.g. 1-click liquid staking)
		type StakeibcPacketMetadata struct {

	im.keeper.Logger(ctx).Error(fmt.Sprintf("Error redeem staking packet from autopilot for %s: %s", newData.Sender, err.Error()))
	im.keeper.Logger(ctx).Debug("failed redeem staking packet from autopilot", "sender" newData.Sender.String(), "error", err.Error())

		voucherPrefix := transfertypes.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())
		stAssetDenom := newData.Denom[len(voucherPrefix):]

		@@ -57,10 +51,6 @@ func (m StakeibcPacketMetadata) Validate() error {

		// Validate claim packet metadata includes the stride address
		func (m ClaimPacketMetadata) Validate() error {

Autopilot enhancement #771

Autopilot enhancement #771

Conversation

jstr1121 commented May 12, 2023 • edited Loading

Context and purpose of the change

Brief Changelog

Author's Checklist

Reviewers Checklist

How to test integration test on local

How to test Hub -> Stride -> Osmosis

Documentation and Release Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 9, 2023

github-actions bot commented Jul 12, 2023

github-actions bot commented Aug 16, 2023

riley-stride left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 8, 2023

Choose a reason for hiding this comment

jstr1121 commented May 12, 2023 •

edited

Loading