Skip to content

imp(transfer)!: use AddressCodec instead of Bech32 (backport #8573)#8616

Closed
gjermundgaraba wants to merge 2 commits intorelease/v10.3.xfrom
gjermund/yihuang/backport-8573
Closed

imp(transfer)!: use AddressCodec instead of Bech32 (backport #8573)#8616
gjermundgaraba wants to merge 2 commits intorelease/v10.3.xfrom
gjermund/yihuang/backport-8573

Conversation

@gjermundgaraba
Copy link
Contributor

Co-authored-by: Gjermund Garaba gjermund@garaba.net

Description

closes: #XXXX


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.

  • Linked to GitHub issue with discussion and accepted design, OR link to spec that describes this work.
  • Include changelog entry when appropriate (e.g. chores should be omitted from changelog).
  • Wrote unit and integration tests if relevant.
  • Updated documentation (docs/) if anything is changed.
  • Added godoc comments if relevant.
  • Self-reviewed Files changed in the GitHub PR explorer.
  • Provide a conventional commit message to follow the repository standards.
  • imp: added address codec to transfer

  • imp: address codec used in transfer

  • fix: tests

  • imp: test passing

  • imp: added addressCodec to PFM

  • imp: added receiver test

  • test: transfer cov

  • test: added cases

  • test: more cov

  • test: more cov

  • style: single line func def

  • test: new test works

  • imp: add basic test

  • imp: added migration guide

  • doc: added changelog

  • refactor: address codec moved

* imp: added address codec to transfer

* imp: address codec used in transfer

* fix: tests

* imp: test passing

* imp: added addressCodec to PFM

* imp: added receiver test

* test: transfer cov

* test: added cases

* test: more cov

* test: more cov

* style: single line func def

* test: new test works

* imp: add basic test

* imp: added migration guide

* doc: added changelog

* refactor: address codec moved
@mergify
Copy link
Contributor

mergify bot commented Aug 28, 2025

⚠️ The sha of the head commit of this PR conflicts with #8615. Mergify cannot evaluate rules on this PR. ⚠️

Comment on lines +130 to 138
var receiver sdk.AccAddress
var err error
if k.addressCodec != nil {
receiver, err = k.addressCodec.StringToBytes(data.Receiver)
} else {
receiver, err = sdk.AccAddressFromBech32(data.Receiver)
}
if err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address: %s", data.Receiver)
Copy link

Choose a reason for hiding this comment

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

The error handling is inconsistent between the original and new code paths. In the original implementation, AccAddressFromBech32 failures were wrapped with the message "failed to decode receiver address". In the new implementation, this error wrapping only occurs after both addressCodec.StringToBytes and AccAddressFromBech32 have failed.

Consider standardizing the error handling by wrapping errors from both code paths consistently:

var receiver sdk.AccAddress
var err error
if k.addressCodec != nil {
    receiver, err = k.addressCodec.StringToBytes(data.Receiver)
    if err != nil {
        return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address: %s", data.Receiver)
    }
} else {
    receiver, err = sdk.AccAddressFromBech32(data.Receiver)
    if err != nil {
        return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address: %s", data.Receiver)
    }
}

This ensures consistent error messages regardless of which address decoding method is used.

Suggested change
var receiver sdk.AccAddress
var err error
if k.addressCodec != nil {
receiver, err = k.addressCodec.StringToBytes(data.Receiver)
} else {
receiver, err = sdk.AccAddressFromBech32(data.Receiver)
}
if err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address: %s", data.Receiver)
var receiver sdk.AccAddress
var err error
if k.addressCodec != nil {
receiver, err = k.addressCodec.StringToBytes(data.Receiver)
if err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address: %s", data.Receiver)
}
} else {
receiver, err = sdk.AccAddressFromBech32(data.Receiver)
if err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address: %s", data.Receiver)
}
}

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@mergify
Copy link
Contributor

mergify bot commented Aug 29, 2025

⚠️ The sha of the head commit of this PR conflicts with #8615. Mergify cannot evaluate rules on this PR. ⚠️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants