Skip to content
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.

Sign up for GitHub

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

Jump to bottom

imp: replace Sender/ReceiverChainIsSource with HasPrefix #6705

Merged
merged 164 commits into from
Jun 28, 2024
Merged

imp: replace Sender/ReceiverChainIsSource with HasPrefix #6705

merged 164 commits into from
Jun 28, 2024

Conversation

crodriguezvega
Copy link
Contributor

Description

ref: #6696

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against the correct branch (see CONTRIBUTING.md).
  • Linked to GitHub issue with discussion and accepted design, OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/).
  • Added relevant godoc comments.
  • Provide a conventional commit message to follow the repository standards.
  • Include a descriptive changelog entry when appropriate. This may be left to the discretion of the PR reviewers. (e.g. chores should be omitted from changelog)
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Review SonarCloud Report in the comment section below once CI passes.

chatton and others added 30 commits April 8, 2024 13:00
@chatton
Adding proto files for ics20-v2 (#6110)
c07bca9 
* chore: adding proto files for ics20-v2

* chore: add newline
@charleenfei
update amount -> string (#6120)
e66bd89
@chatton
Update MsgTransfer to accept sdk.Coins instead of sdk.Coin (#6113)
034f472
@crodriguezvega
fix: allow base denom with trailing slash (#6148)
4cc6a85
@charleenfei
imp: add CurrentVersion, EscrowVersion (#6160)
71f830c 
* add CurrentVersion, EscrowVersion, update tests

* update hardcoded transfer channel version from interchaintest
@chatton @charleenfei
chore: add function for converting packet data from v1 to v3 (#6116)
28ff9b6 
---------

Co-authored-by: Charly <charly@interchain.berlin>
@charleenfei
chore: implement required FungibleTokenPacketData v3 interface meth…
4e55137 
…ods (#6126)
@charleenfei @chatton @crodriguezvega
imp: getMultiDenomFungibleTokenPacketDatato be used in packet unmar…
ca056cf 
…shalling/conversion (#6226)

* chore: adding proto files for ics20-v2

* chore: add newline

* chore: modify MsgTransfer to accept coins instead of coin

* chore: reverted unintentional comment changes

* chore: reverted unintentional comment changes

* chore: adding test for conversion fn

* chore: fix e2e linter

* chore: adding docs

* chore: addressing PR feedback

* chore: remove duplicate import

* chore: addressing PR feedback

* chore: moved extration logic into internal package

* chore: commented out failing test

* chore: adding link to issue

* chore: remove duplicate import

* chore: fix linting errors

* FungibleTokenPacketData interface methods + tests

* linter

* wip: token validation

* update trace identifier validation in Token + tests

* rm Printf

* update with pr review

* add CurrentVersion, EscrowVersion, update tests

* pr review

* fix e2e tests

* pr review

* update e2e test version

* linter

* update hardcoded transfer channel version from interchaintest

* update hardcoded transfer channel version from interchaintest

* wip packet unmarshaller

* wip

* wip testing

* update test

* linter

* rm unnecessary version changes

* rm unnecessary artifacts

* update callbacks test

* update comment

* pr review

* rename getMultiDenomFungibleTokenPacketData to unmarshalPacketDataBytesToICS20V2

---------

Co-authored-by: chatton <github.qpeyb@simplelogin.fr>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
@charleenfei @crodriguezvega
chore: implement version checking for channel handshake application c…
147cf17 
…allbacks (#6175)

* add SupportedVersions array for different ics20 versions, add version checking on channel handshake application callbacks

* add tests

* update pr review

* pr review

* last few pr review nits

* linter

* add version counter proposing

* fix missing app versino

* update code + tests to return our proposed version if counterparty version is invalid

* remove if statement

* address review comments: return ics20-2 if counterparty version is not supported

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
@charleenfei @crodriguezvega
imp: update transfer authz implementation to account for multi denom …
9bbfa1a 
…transfers (#6252)

* add transfer authz code + tests

* linter

* address review comments

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
@crodriguezvega @chatton @charleenfei
ics20-v2: backwards compatibility for transfer rpc and packet callbac…
4f57916 
…ks (#6189)

* chore: adding proto files for ics20-v2

* chore: add newline

* chore: modify MsgTransfer to accept coins instead of coin

* chore: reverted unintentional comment changes

* chore: reverted unintentional comment changes

* chore: adding test for conversion fn

* chore: fix e2e linter

* chore: adding docs

* chore: addressing PR feedback

* chore: remove duplicate import

* chore: addressing PR feedback

* chore: moved extration logic into internal package

* chore: commented out failing test

* chore: adding link to issue

* chore: remove duplicate import

* chore: fix linting errors

* FungibleTokenPacketData interface methods + tests

* linter

* wip: token validation

* update trace identifier validation in Token + tests

* rm Printf

* update with pr review

* pr review

* linter

* rm unused function: linter

* wip pr feedback

* fix test

* pr review

* lintttttt

* wip: backwards compatibility for transfer rpc

* implement changes for ics20-v2 packet data for onRecvPacket, onAcknowledgePacket and onTimeoutPacket

* fix callbacks tests

* lint

---------

Co-authored-by: chatton <github.qpeyb@simplelogin.fr>
Co-authored-by: Charly <charly@interchain.berlin>
@sangier
add v3 packet proto
e04047e
@sangier
fix protos
c73d5f6
@sangier
fixes
03cc2e5
@sangier
test fixes
65f4476
@sangier
add forwardPath keys and memo check in sendTransfer
db85a29
@sangier
wip onRecvPacket logic
999ed80
@sangier
minor fixes
be72180
@crodriguezvega
changes to transfer tx CLI to support multiple denoms
4c333c5
@crodriguezvega
import renaming
0478cb9
@sangier
onRecv logic completed
37f5b5e
@sangier
add revertInFlights function
3fd3345
@sangier
add onAck && onTimeout logic
24e2407
@crodriguezvega
fix interchain accounts test
7858528
@crodriguezvega
basic unit test for path forwarding
55d7bf9
@sangier
fix test unsuccessful refund from source
1a95262
@sangier
wip test fix
44e15cb
@sangier
fix mbt test - need more investigation
334d5a9
@sangier
revert test fix
7466b19
@sangier
add assertions
5c46d79
colin-axner
colin-axner approved these changes Jun 27, 2024
View reviewed changes
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for updating the if else, I think it's more natural to reason about

modules/apps/transfer/keeper/relay.go Outdated Show resolved Hide resolved
crodriguezvega and others added 16 commits June 27, 2024 12:33
@crodriguezvega
Update CHANGELOG.md
7afb7f6
@crodriguezvega
Update CHANGELOG.md
0221c6b
@crodriguezvega
make getForwardedPacket private
cc46a63
@crodriguezvega
remove auxiliary burn coins function
e67aeaf
@damiannolan
nit: rename func method recv args in types/forwarding.go
966a644
@damiannolan
chore: rename ShouldBeForwarded to HasFowarding
e92e086
@DimitrisJim
e2e: remove template test for three chain setup.
6ea614f
@DimitrisJim
nit: no generics silly
18d0567
@DimitrisJim
nit: add clarifying comment to validate basic call on msg.
1b7214d
@DimitrisJim
nit: remove unused key.
1f5e61f
@DimitrisJim
nit: clean up cli help text.
0a78bde
@DimitrisJim
nit: don't export is blocked address helper.
07036a9
@DimitrisJim
nit: docustring for e2e test and helper.
1442874
@DimitrisJim @damiannolan
nit: improve documentation for transfer's OnRecv callback.
25c73c4 
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
@crodriguezvega @colin-axner
Update modules/apps/transfer/keeper/relay.go
8212cf1 
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
@crodriguezvega
Merge branch 'feat/ics20-v2-path-forwarding' into carlos/implement-de…
167081a 
…nom-hasprefix
chatton
chatton approved these changes Jun 27, 2024
View reviewed changes
Copy link
Contributor

@chatton chatton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! This reads a good bit better now i think, thanks for the cleanup 💪

@crodriguezvega crodriguezvega changed the base branch from feat/ics20-v2-path-forwarding to main June 28, 2024 11:00
@crodriguezvega
Merge branch 'main' into carlos/implement-denom-hasprefix
7d21340 
# Conflicts:
#	modules/apps/transfer/client/cli/tx.go
#	modules/apps/transfer/keeper/forwarding.go
#	modules/apps/transfer/keeper/relay.go
#	modules/apps/transfer/keeper/relay_forwarding_test.go
#	modules/apps/transfer/types/forwarding.go
#	modules/apps/transfer/types/transfer_authorization.go
@crodriguezvega
delete unused file
722d825
@crodriguezvega
Merge branch 'main' into carlos/implement-denom-hasprefix
8208427
@crodriguezvega
Merge branch 'main' into carlos/implement-denom-hasprefix
c782e62
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jun 28, 2024

Quality Gate Passed Quality Gate passed for 'ibc-go'

Issues
13 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@crodriguezvega crodriguezvega merged commit b1743e0 into main Jun 28, 2024
85 checks passed
@crodriguezvega crodriguezvega deleted the carlos/implement-denom-hasprefix branch June 28, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colin-axner colin-axner colin-axner approved these changes

@DimitrisJim DimitrisJim DimitrisJim approved these changes

@chatton chatton chatton approved these changes

@AdityaSripal AdityaSripal Awaiting requested review from AdityaSripal AdityaSripal is a code owner

@damiannolan damiannolan Awaiting requested review from damiannolan damiannolan is a code owner

@srdtrk srdtrk Awaiting requested review from srdtrk srdtrk is a code owner

@gjermundgaraba gjermundgaraba Awaiting requested review from gjermundgaraba

@bznein bznein Awaiting requested review from bznein

Assignees
No one assigned
Labels
priority PRs that need prompt reviews
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.