Autopilot Channel Verification #728

sampocs · 2023-04-19T06:14:23Z

Closes: #XXX

Context and purpose of the change

Brief Changelog

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

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

…ng unit test

riley-stride

Looks pretty good, requesting a couple new tests

x/autopilot/keeper/airdrop.go

x/autopilot/keeper/airdrop_test.go

shellvish

Looks great, a few small comments but should be very quick. Excellent test coverage. Thank you for doing this!

shellvish · 2023-04-21T15:09:40Z

dockernet/scripts/airdrop/README.md

@@ -0,0 +1,23 @@
+## Airdrop Integration Tests
+Each airdrop testing script (1 through 4) tests differnet aspects of the airdrop. 


extreme nit: differnet -> different

dockernet/scripts/airdrop/airdrop1.sh

shellvish · 2023-04-21T15:16:42Z

dockernet/scripts/airdrop/airdrop1.sh

 sleep 5

 # AIRDROP CLAIMS
 # Check balances before claims
-echo "Initial balance before claim:"
-$STRIDE_MAIN_CMD query bank balances stride1nf6v2paty9m22l3ecm7dpakq2c92ueyununayr
+# QUESTION: I assume these tests cannot be run back to back - but if they can then I can change the expectations below to be deltas


I think they actually can be re-run but it requires the airdrop duration (line 38) to be something smaller, like 240s. imo not worth prioritizing this now, given it'll also take time to modify all of the queries, and I don't think we'll be running these back-to-back very often

yup agreed! I think for now we should treat these tests as idempotent scripts that need a dockernet restart each time. And we can always come back and try to make them idempotent later, but low priority!

shellvish · 2023-04-21T15:17:34Z

proto/stride/claim/params.proto

@@ -13,6 +13,7 @@ message Params { repeated Airdrop airdrops = 1; }
 message Airdrop {
  string airdrop_identifier = 1
      [ (gogoproto.moretags) = "yaml:\"airdrop_identifier\"" ];
+  string chain_id = 7;


just curious, why add chain_id here instead of after param 6?

just my OCD taking over haha, It felt like a more logical ordering of the fields, plus it aligns with the ordering of the cli args

x/autopilot/keeper/airdrop.go

x/autopilot/keeper/airdrop_test.go

x/claim/keeper/claim.go

riley-stride · 2023-04-21T17:10:44Z

Resolutions look good to me

asalzmann

couple of minor changes, otherwise lgtm!

dockernet/scripts/airdrop/README.md

x/stakeibc/keeper/host_zone.go

x/claim/types/params.go

x/claim/keeper/claim.go

x/autopilot/keeper/airdrop.go

asalzmann

lgtm

added additional safety check in airdrop autopilot

e00a771

sampocs requested a review from a team April 19, 2023 06:14

github-actions bot added the C:stakeibc label Apr 19, 2023

fixed unit test

fe17412

sampocs changed the title ~~Port & Channel Check in Autopilot~~ Autopilot Channel Verification Apr 19, 2023

added test cases for different port/channel ids

2a6972a

sampocs marked this pull request as ready for review April 19, 2023 06:55

sampocs added 5 commits April 19, 2023 14:08

added chain_id and autopilot enabled fields to airdrop type

73ebe37

added chain_id and autopilot enabled fields to msg create airdrop

5f30d0b

updated CreateAirdropAndEpoch to check the chain-id

0d5afbb

fixed unit tests for create airdrop

2eca370

added more unit tests to create airdrop to check errors

0a432d2

github-actions bot added C:app-wiring C:CLI C:proto labels Apr 19, 2023

sampocs added 6 commits April 19, 2023 16:17

added airdrop lookup by chain-id in autopilot

266b1a8

removed airdrop id from packet metadata and updated unit tests

3e1b85b

added check that autopilot is enabled for the airdrop and correspondi…

b7e5f0c

…ng unit test

updated integration test scripts

fd0456f

nit changes to airdrop scripts

3f0bd06

more airdrop script cleanup

0e19367

sampocs force-pushed the airdrop-channel-check branch from a019426 to 1ea62da Compare April 20, 2023 17:34

added default epoch duration as a param

0f49b0f

sampocs force-pushed the airdrop-channel-check branch from 1ea62da to 0f49b0f Compare April 20, 2023 17:40

finalized integration tests

c4eb338

riley-stride requested changes Apr 21, 2023

View reviewed changes

x/autopilot/keeper/airdrop.go Outdated Show resolved Hide resolved

x/autopilot/keeper/airdrop.go Show resolved Hide resolved

x/autopilot/keeper/airdrop_test.go Outdated Show resolved Hide resolved

addressed riley pr comments

4d9b073

shellvish approved these changes Apr 21, 2023

View reviewed changes

sampocs added 2 commits April 21, 2023 10:54

addressed vishal pr comments

648744c

Merge branch 'main' into airdrop-channel-check

a63f4de

riley-stride approved these changes Apr 21, 2023

View reviewed changes

cleaned up evmos airdrop script

06b6bd8

github-actions bot added the T:build label Apr 21, 2023

sampocs added 2 commits April 21, 2023 13:18

some more nits

d5c226d

renamed scripts and updated docs

1b31e87

asalzmann requested changes Apr 21, 2023

View reviewed changes

added channel invariant check to msg_register_host_zone

0351d75

asalzmann approved these changes Apr 21, 2023

View reviewed changes

nit comment

0c68fa1

sampocs force-pushed the airdrop-channel-check branch from 118df0d to 0c68fa1 Compare April 21, 2023 21:53

sampocs merged commit 62295e3 into main Apr 21, 2023

faddat mentioned this pull request May 8, 2023

refresh 47 #763

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autopilot Channel Verification #728

Autopilot Channel Verification #728

sampocs commented Apr 19, 2023

riley-stride left a comment

shellvish left a comment

shellvish Apr 21, 2023

sampocs Apr 21, 2023

shellvish Apr 21, 2023

sampocs Apr 21, 2023

shellvish Apr 21, 2023

sampocs Apr 21, 2023

riley-stride commented Apr 21, 2023

asalzmann left a comment

asalzmann left a comment

		@@ -0,0 +1,23 @@
		## Airdrop Integration Tests
		Each airdrop testing script (1 through 4) tests differnet aspects of the airdrop.

Autopilot Channel Verification #728

Autopilot Channel Verification #728

Conversation

sampocs commented Apr 19, 2023

Context and purpose of the change

Brief Changelog

Author's Checklist

Reviewers Checklist

Documentation and Release Note

riley-stride left a comment

Choose a reason for hiding this comment

shellvish left a comment

Choose a reason for hiding this comment

shellvish Apr 21, 2023

Choose a reason for hiding this comment

sampocs Apr 21, 2023

Choose a reason for hiding this comment

shellvish Apr 21, 2023

Choose a reason for hiding this comment

sampocs Apr 21, 2023

Choose a reason for hiding this comment

shellvish Apr 21, 2023

Choose a reason for hiding this comment

sampocs Apr 21, 2023

Choose a reason for hiding this comment

riley-stride commented Apr 21, 2023

asalzmann left a comment

Choose a reason for hiding this comment

asalzmann left a comment

Choose a reason for hiding this comment