-
Notifications
You must be signed in to change notification settings - Fork 586
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.
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
(test) Add test for more than one forwarding hop #6713
Merged
Merged
Changes from all commits
Commits
Show all changes
176 commits
Select commit
Hold shift + click to select a range
c07bca9
Adding proto files for ics20-v2 (#6110)
chatton e66bd89
update amount -> string (#6120)
charleenfei 034f472
Update MsgTransfer to accept sdk.Coins instead of sdk.Coin (#6113)
chatton 4cc6a85
fix: allow base denom with trailing slash (#6148)
crodriguezvega 71f830c
imp: add CurrentVersion, EscrowVersion (#6160)
charleenfei 28ff9b6
chore: add function for converting packet data from v1 to v3 (#6116)
chatton 4e55137
chore: implement required `FungibleTokenPacketData` v3 interface meth…
charleenfei ca056cf
imp: `getMultiDenomFungibleTokenPacketData`to be used in packet unmar…
charleenfei 147cf17
chore: implement version checking for channel handshake application c…
charleenfei 9bbfa1a
imp: update transfer authz implementation to account for multi denom …
charleenfei 4f57916
ics20-v2: backwards compatibility for transfer rpc and packet callbac…
crodriguezvega e04047e
add v3 packet proto
sangier c73d5f6
fix protos
sangier 03cc2e5
fixes
sangier 65f4476
test fixes
sangier db85a29
add forwardPath keys and memo check in sendTransfer
sangier 999ed80
wip onRecvPacket logic
sangier be72180
minor fixes
sangier 4c333c5
changes to transfer tx CLI to support multiple denoms
crodriguezvega 0478cb9
import renaming
crodriguezvega 37f5b5e
onRecv logic completed
sangier 3fd3345
add revertInFlights function
sangier 24e2407
add onAck && onTimeout logic
sangier 7858528
fix interchain accounts test
crodriguezvega 55d7bf9
basic unit test for path forwarding
crodriguezvega 1a95262
fix test unsuccessful refund from source
sangier 44e15cb
wip test fix
sangier 334d5a9
fix mbt test - need more investigation
sangier 7466b19
revert test fix
sangier 5c46d79
add assertions
sangier 99e6b0a
add support for async ack
sangier a114af6
wip test forwarding happy path
sangier d800caa
icsv20(path forwarding): use nil as default forwarding path when not …
crodriguezvega dd873a1
add forwarding happy path tests
sangier a292a3f
Merge branch 'feat/ics20-v2' into stefano/carlos/ics20-v2-forwarding-poc
sangier e25ba0c
fix merge
sangier f39d173
Use type with V2 suffix for package data (#6330)
chatton 9b39944
Adding additional comments and changing version variable names (#6345)
chatton 06ca9a5
Merge branch 'main' into merge-main
chatton 7897ef3
chore: correctly claim capability
chatton 786a4f1
lint
colin-axner 5747756
Merge pull request #6346 from cosmos/merge-main
DimitrisJim 7e2e6df
Merge branch 'main' into feat/ics20-v2
chatton a84b0e7
imp: change ics20 events to emit token set (#6348)
colin-axner 43877df
imp: check length tokens array against maximum allowed (#6349)
crodriguezvega 8eae033
Modify UnmarshalPacketData interface to allow additional args (#6341)
DimitrisJim dbcff45
Refactor packet data unmarshalling to use specific version (#6354)
chatton bb69698
Merge branch 'main' into merge-main-2
chatton f19a145
chore: fixing tests
chatton 8f86dda
Merge pull request #6359 from cosmos/merge-main-2
chatton d4b06c8
imp: self review comments for ics20-v2 (#6360)
colin-axner a9391a4
imp: self review on ics20-v2 part 2 (#6364)
colin-axner 575403e
chore: move functions from internal/denom back to trace.go (#6368)
DimitrisJim 50ccd94
imp: ics20 v2 self review part 3 (#6373)
colin-axner 87eb32e
chore: remove duplicate test case
colin-axner e8b9d5a
chore: address minor nits (#6374)
DimitrisJim 57aab01
Merge branch 'feat/ics20-v2' into stefano/carlos/ics20-v2-forwarding-poc
crodriguezvega 2519593
fix lint warning, add extra godocs, and some other small fixes and cl…
crodriguezvega 3bf6c04
Merge branch 'main' into stefano/carlos/ics20-v2-forwarding-poc
crodriguezvega 64c2fc4
fix finalReceiver address bug
sangier 7cd7260
wip - ack test scenario5
sangier 6673e74
add FungibleTokenPacketDataV2 test for ValidateBasic (#6398)
hastur199 550eeef
fix linter complaints
crodriguezvega 4c54a88
add test - currently faling on middle hop revert
sangier debc4cb
add test comments
sangier e50fa40
fixes
sangier 6d87d95
Merge branch 'main' into stefano/carlos/ics20-v2-forwarding-poc
crodriguezvega 7c8f516
retrieve channel capability only if there is a previous packet in store
crodriguezvega feb7b01
Merge branch 'main' into stefano/carlos/ics20-v2-forwarding-poc
crodriguezvega 6fd1773
Merge branch 'main' into stefano/carlos/ics20-v2-forwarding-poc
crodriguezvega efcfa5d
add missing parameter
crodriguezvega 21dbb37
fix: e2e build failures.
DimitrisJim 70d7a29
Use Transfer instead of sendTransfer when forwarding. (#6564)
DimitrisJim 465b16c
lint: fix linter issues.
DimitrisJim ec472cf
tests(transfer): move forwarding tests to separate file. (#6568)
DimitrisJim e483b9a
chore: rename ForwardingInfo to Forwarding
damiannolan 0c9f368
Revert "chore: rename ForwardingInfo to Forwarding"
damiannolan 413b7c1
nit(transfer): Mark hops as non nullable. (#6566)
DimitrisJim ae2046a
feat(transfer): add forwarding info validation to token packet (#6571)
gjermundgaraba ba7c4a8
feat(transfer): add validation for forwarding info in msg transfer va…
gjermundgaraba 6b3b5aa
Fix and simplify reverts of forwarding state (#6574)
srdtrk 5070e50
chore: rename ForwardingInfo to Forwarding (#6576)
damiannolan d874b54
Refactor packet forward functions (#6575)
chatton a8cd302
Merge branch 'main' into feat/ics20-v2-path-forwarding
crodriguezvega fbb9cd8
feat(transfer): validate forwarding memo in transfer authorization (#…
gjermundgaraba c4987a7
Add func convert token to coin ibc (#6584)
duonghb53 a90b671
transfer: Disallow a forwarding object specified with zero hops and a…
DimitrisJim c348732
feat(transfer): move async decision and handling to the ibc module on…
gjermundgaraba 789b09b
Merge branch 'main' into feat/ics20-v2-path-forwarding
crodriguezvega c00ddd9
chore: use NewForwarding instead of direct init (#6605)
gjermundgaraba fc6c111
Reduce max forwarding to 16 (#6610)
DimitrisJim 72714b3
feat(transfer): use single byte ack for successful forward (#6604)
gjermundgaraba c546f69
chore(transfer/cli): add forwarding flag to tx cli (#6609)
DimitrisJim 60a6b99
chore(transfer): make Forwarding non-null (#6618)
DimitrisJim dc47641
chore: restructure functions with logical ordering (#6638)
damiannolan 832b1bd
test: Add tests for OnTimeoutPacket when middle chain times out packe…
bznein 360b2f8
feat(transfer): add ShouldBeForwarded convenience method to msg trans…
gjermundgaraba 84c7c33
disallow timeout height usage when forwarding packets (#6641)
crodriguezvega 007dee1
nit: make set forwarded packet unexported (#6637)
crodriguezvega 14d5486
feat(transfer): use registered error code for error acks in token for…
gjermundgaraba a28a549
chore(transfer): emit forwarding information in events. (#6647)
DimitrisJim d6db0c5
Merge branch 'main' into gjermund/merge-main-to-feat-ics20-v2-path-fo…
gjermundgaraba 64bc502
Fix e2e test
gjermundgaraba 1ca1f2f
Merge pull request #6662 from cosmos/gjermund/merge-main-to-feat-ics2…
chatton 87d1f91
Refactor forwarding messages for Transfer and Packet (#6655)
DimitrisJim ef6d22f
feat: allow authz granters to specify forwarding info for token trans…
bznein da2f9f6
feat: delete forwarded packet when it is not needed anymore (#6621)
bznein e68143b
test(transfer): forwarding acknowledgment errors in middle hop (#6659)
gjermundgaraba 6619821
test(transfer): last chain in forwarding packet is ICS20 v1 (#6622)
gjermundgaraba 43eceed
refactor: rename SetupPath to SetupPaths (#6674)
gjermundgaraba aa860ac
chore: add flag for unwind in transfer cli (#6680)
neitdung 2b4d24b
feat: impl check reject transfer if len(hops) > 0 and ics20-1 (#6675)
duonghb53 59e3df7
feat(transfer): add unwinding ability (#6656)
DimitrisJim e3ffcff
Merge branch 'main' into feat/ics20-v2-path-forwarding
chatton 9c5ae03
fix typo
crodriguezvega c2a6bc6
remove unnecessary wrapping of function
crodriguezvega c3ccbfc
Revert "remove unnecessary wrapping of function"
crodriguezvega 5345776
fix usage of function
crodriguezvega 74088ba
(chore) replace reflect.DeepEqual with slices.Equal (#6697)
bznein 272c12b
chore: comment hop slicing for clarity (#6702)
gjermundgaraba 4349a1d
chore: cleanup forwarding tests (#6691)
crodriguezvega 24cd07f
chore: pull out hop validation and consolidate for transfer+packet (#…
gjermundgaraba dca8c42
Merge branch 'main' into feat/ics20-v2-path-forwarding
chatton 37d7335
Remove unwind field in authz (#6701)
chatton 6e1a082
chore: add packet data validation back (#6704)
chatton 8f9691f
(chore) Refactor code around forwarding validation (#6706)
bznein c505243
use setupForwardingPaths in test
crodriguezvega d715455
feat(transfer): allow non-cosmos-sdk AccAddress in final receiver for…
gjermundgaraba 4e61ea9
chore: pass only hops to sendTransfer + events rename (#6703)
gjermundgaraba 3e2a5fa
test: forwarding test that verifies forwarded memo (#6707)
gjermundgaraba 5ea9d79
chore: update godoc for relay forwarding tests
gjermundgaraba 100ccc7
chore: use module account instead of custom forward address (#6688)
gjermundgaraba 9526592
Test for four chains!
bznein 13d3ac6
chore: replace continue with if/else (#6700)
crodriguezvega 1e25ae8
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
bznein b6d77d7
Merge branch 'main' into feat/ics20-v2-path-forwarding
crodriguezvega 4d19be1
add changelog
crodriguezvega 82b83d6
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
bznein 9294bab
Propagate ack.
bznein c2707bf
Check error
bznein 601c4ed
Removed helper
bznein 2838aef
add test for invalid receiver address
crodriguezvega 7b43e03
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
bznein 7afb7f6
Update CHANGELOG.md
crodriguezvega 0221c6b
Update CHANGELOG.md
crodriguezvega 75cdc94
Update CHANGELOG.md
crodriguezvega a799f23
Update CHANGELOG.md
crodriguezvega cc46a63
make getForwardedPacket private
crodriguezvega e67aeaf
remove auxiliary burn coins function
crodriguezvega 966a644
nit: rename func method recv args in types/forwarding.go
damiannolan e92e086
chore: rename ShouldBeForwarded to HasFowarding
damiannolan 6ea614f
e2e: remove template test for three chain setup.
DimitrisJim 18d0567
nit: no generics silly
DimitrisJim 1b7214d
nit: add clarifying comment to validate basic call on msg.
DimitrisJim 1f5e61f
nit: remove unused key.
DimitrisJim 0a78bde
nit: clean up cli help text.
DimitrisJim 07036a9
nit: don't export is blocked address helper.
DimitrisJim 1442874
nit: docustring for e2e test and helper.
DimitrisJim 25c73c4
nit: improve documentation for transfer's OnRecv callback.
DimitrisJim 03f6278
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
crodriguezvega baf3fd3
Apply suggestions from code review
crodriguezvega 7d2a881
chore: remove unused function
crodriguezvega 4ea36c2
perf: allocate slice to length of packet data tokens
crodriguezvega 0a28e9b
Merge branch 'main' into feat/ics20-v2-path-forwarding
crodriguezvega 2c5ff48
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
bznein 7411f2d
chore(transfer/authz): wrapf unauthorized forwarding hops
damiannolan 199c14a
lint
crodriguezvega c7af85a
Update modules/apps/transfer/types/forwarding.go
crodriguezvega 25239a2
Merge branch 'main' into feat/ics20-v2-path-forwarding
crodriguezvega de65749
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
bznein 90d57db
Preallocate slice but keep len==0 (#6725)
bznein e310e27
imp: validate allowed forwarding hops
crodriguezvega f28e937
test: unwind fails in Transfer rpc
crodriguezvega 50daed0
Merge branch 'main' into feat/ics20-v2-path-forwarding
crodriguezvega 5e84955
Merge branch 'feat/ics20-v2-path-forwarding' into bznein/fourchainstest
bznein b7604e8
Merge branch 'main' into bznein/fourchainstest
bznein File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1029,3 +1029,119 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() { | |
// And that A has its original balance back. | ||
suite.assertAmountOnChain(suite.chainA, balance, originalABalance.Amount, coin.Denom) | ||
} | ||
|
||
// TestForwardingWithMoreThanOneHop tests the scenario in which we | ||
// forward with more than one forwarding hop. | ||
func (suite *KeeperTestSuite) TestForwardingWithMoreThanOneHop() { | ||
// Setup A->B->C->D | ||
coinOnA := ibctesting.TestCoin | ||
|
||
pathAtoB := ibctesting.NewTransferPath(suite.chainA, suite.chainB) | ||
pathAtoB.Setup() | ||
|
||
pathBtoC := ibctesting.NewTransferPath(suite.chainB, suite.chainC) | ||
pathBtoC.Setup() | ||
|
||
pathCtoD := ibctesting.NewTransferPath(suite.chainC, suite.chainD) | ||
pathCtoD.Setup() | ||
|
||
sender := suite.chainA.SenderAccounts[0].SenderAccount | ||
receiver := suite.chainD.SenderAccounts[0].SenderAccount | ||
|
||
forwarding := types.NewForwarding(false, | ||
types.Hop{PortId: pathBtoC.EndpointA.ChannelConfig.PortID, ChannelId: pathBtoC.EndpointA.ChannelID}, | ||
types.Hop{PortId: pathCtoD.EndpointA.ChannelConfig.PortID, ChannelId: pathCtoD.EndpointA.ChannelID}, | ||
) | ||
|
||
transferMsg := types.NewMsgTransfer( | ||
pathAtoB.EndpointA.ChannelConfig.PortID, | ||
pathAtoB.EndpointA.ChannelID, | ||
sdk.NewCoins(coinOnA), | ||
sender.GetAddress().String(), | ||
receiver.GetAddress().String(), | ||
clienttypes.ZeroHeight(), | ||
suite.chainA.GetTimeoutTimestamp(), | ||
"", | ||
forwarding) | ||
|
||
// Send message to A and verify. | ||
result, err := suite.chainA.SendMsgs(transferMsg) | ||
suite.Require().NoError(err) | ||
|
||
packetFromAtoB, err := ibctesting.ParsePacketFromEvents(result.Events) | ||
suite.Require().NoError(err) | ||
suite.Require().NotNil(packetFromAtoB) | ||
|
||
err = pathAtoB.EndpointB.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
// Receive from B and verify. | ||
result, err = pathAtoB.EndpointB.RecvPacketWithResult(packetFromAtoB) | ||
suite.Require().NoError(err) | ||
suite.Require().NotNil(result) | ||
|
||
// Check that Escrow A has amount | ||
suite.assertAmountOnChain(suite.chainA, escrow, coinOnA.Amount, coinOnA.Denom) | ||
|
||
// Check that Escrow B has amount | ||
denomTrace := types.NewDenom(sdk.DefaultBondDenom, types.NewTrace(pathAtoB.EndpointB.ChannelConfig.PortID, pathAtoB.EndpointB.ChannelID)) | ||
suite.assertAmountOnChain(suite.chainB, escrow, coinOnA.Amount, denomTrace.IBCDenom()) | ||
|
||
// Receive on C the packet sent from B, verify amount. | ||
packetFromBtoC, err := ibctesting.ParsePacketFromEvents(result.Events) | ||
suite.Require().NoError(err) | ||
suite.Require().NotNil(packetFromBtoC) | ||
|
||
err = pathBtoC.EndpointA.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
err = pathBtoC.EndpointB.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
result, err = pathBtoC.EndpointB.RecvPacketWithResult(packetFromBtoC) | ||
suite.Require().NoError(err) | ||
suite.Require().NotNil(result) | ||
|
||
// Check that Escrow C has amount | ||
denomTraceABC := types.NewDenom(denomTrace.Base, append([]types.Trace{types.NewTrace(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID)}, denomTrace.Trace...)...) | ||
suite.assertAmountOnChain(suite.chainC, escrow, coinOnA.Amount, denomTraceABC.IBCDenom()) | ||
|
||
// Finally, receive on D and verify that D has the desired amount. | ||
packetFromCtoD, err := ibctesting.ParsePacketFromEvents(result.Events) | ||
suite.Require().NoError(err) | ||
suite.Require().NotNil(packetFromCtoD) | ||
|
||
err = pathCtoD.EndpointA.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
err = pathCtoD.EndpointB.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
result, err = pathCtoD.EndpointB.RecvPacketWithResult(packetFromCtoD) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we also ack the packet back up the stack? ack, err := ibctesting.ParseAckFromEvents(result.Events)
suite.Require().NoError(err)
err = pathCtoD.EndpointA.AcknowledgePacket(packetFromCtoD, ack)
suite.Require().NoError(err)
// etc... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! |
||
suite.Require().NoError(err) | ||
suite.Require().NotNil(result) | ||
|
||
denomTraceABCD := types.NewDenom(denomTraceABC.Base, append([]types.Trace{types.NewTrace(pathCtoD.EndpointB.ChannelConfig.PortID, pathCtoD.EndpointB.ChannelID)}, denomTraceABC.Trace...)...) | ||
suite.assertAmountOnChain(suite.chainD, balance, coinOnA.Amount, denomTraceABCD.IBCDenom()) | ||
|
||
// Propagate the ack back from D to A. | ||
ack, err := ibctesting.ParseAckFromEvents(result.Events) | ||
suite.Require().NoError(err) | ||
suite.Require().NotNil(ack) | ||
|
||
err = pathCtoD.EndpointA.AcknowledgePacket(packetFromCtoD, ack) | ||
suite.Require().NoError(err) | ||
|
||
err = pathBtoC.EndpointA.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
err = pathBtoC.EndpointA.AcknowledgePacket(packetFromBtoC, ack) | ||
suite.Require().NoError(err) | ||
|
||
err = pathAtoB.EndpointA.UpdateClient() | ||
suite.Require().NoError(err) | ||
|
||
err = pathAtoB.EndpointA.AcknowledgePacket(packetFromAtoB, ack) | ||
suite.Require().NoError(err) | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some food for thought for a future refactor or cleanup.. Maybe we should consider a
ForwardingTestSuite
for forwarding tests, since spinning up 4 in process apps is wasteful for most other transfer tests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 we generally do a lot of re-use of the suites and it can get unnecessarily bloatful especially keeper ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a very reasonable take. I'll create an issue for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I think a new test suite for this will be much cleaner