feat(denommetadata): register IBC denom on transfer

[zale144]

Description

---

Closes #955

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.

PR review checkboxes:

I have...

• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Targeted PR against the correct branch
• included the correct type prefix in the PR title
• Linked to the GitHub issue with discussion and accepted design
• Targets only one GitHub issue
• Wrote unit and integration tests
• Wrote relevant migration scripts if necessary
• All CI checks have passed
• Added relevant godoc comments
• Updated the scripts for local run, e.g genesis_config_commands.sh if the PR changes parameters
• Add an issue in the e2e-tests repo if necessary

SDK Checklist

• Import/Export Genesis
• Registered Invariants
• Registered Events
• Updated openapi.yaml
• No usage of go map
• No usage of time.Now()
• Used fixed point arithmetic and not float arithmetic
• Avoid panicking in Begin/End block as much as possible
• No unexpected math Overflow
• Used sendCoin and not SendCoins
• Out-of-block compute is bounded
• No serialized ID at the end of store keys
• UInt to byte conversion should use BigEndian

Full security checklist here

----;

For Reviewer:

• Confirmed the correct type prefix in the PR title
• Reviewers assigned
• Confirmed all author checklist items have been addressed

---;

After reviewer approval:

• In case the PR targets the main branch, PR should not be squash merge in order to keep meaningful git history.
• In case the PR targets a release branch, PR must be rebased.

[mtsitrin]

UT fails

[danwt]

Please, instead of reintroducing all the old stuff, just move the ibc module and ics4 wrapper from transferinject and put your changes in there

If you look at main, currently, all the middlewares are very consistent in their pattern and use the authenticate packet utils

dymension/x/rollapp/keeper/authenticate_packet.go

Line 21 in 4565f34

func (k Keeper) GetValidTransfer(

It's all been dried out

Also please for errors, now prefer to use our gerr cosmos package and wrap them instead of defining new errors a lot. (You can still define new errors, but it's rare to need to do so)

https://pkg.go.dev/github.com/dymensionxyz/[email protected]/gerrc#pkg-variables

[zale144]

Please, instead of reintroducing all the old stuff, just move the ibc module and ics4 wrapper from transferinject and put your changes in there

If you look at main, currently, all the middlewares are very consistent in their pattern and use the authenticate packet utils

dymension/x/rollapp/keeper/authenticate_packet.go

Line 21 in 4565f34

func (k Keeper) GetValidTransfer(

It's all been dried out

Also please for errors, now prefer to use our gerr cosmos package and wrap them instead of defining new errors a lot. (You can still define new errors, but it's rare to need to do so)

https://pkg.go.dev/github.com/dymensionxyz/[email protected]/gerrc#pkg-variables

what do you mean reintroducing all the old stuff? transferinject has been removed with this PR, it's all moved to denommetadata

[danwt]

Looks good man

Please see my comments on the rdk pr and then read this, because they are similar.

Basically, please name the structs IBCModule or ICS4Wrapper, not middleware, because they aren't actually middleware

but basically gj

[danwt]

great job

[mtsitrin]

isn't the rollapp.RegisteredDenoms used for hub->RA denoms registration?
the tokens from RA->Hub supposed to be for RA native tokens.
not sure why u need maintain a list of denoms, as u already set it in the bank module

[zale144]

if we have a situation where the rollapp first registers a denom on the hub, (RA -> HUB), later if the hub wants to send a transfer with the same denom to the rollapp (HUB -> RA), in order to prevent the same denom metadata to be included in the transfer to the rollapp, we add it to rollapp.RegisteredDenoms on the hub when handling OnRecvPacket.

wdym by

the tokens from RA->Hub supposed to be for RA native tokens.

why can't the rollapp register a non-native denom to the hub? The native RA denom is already registered during the "genesis event", or whatever we call it these days.

[mtsitrin]

in order to prevent the same denom metadata to be included in the transfer to the rollapp
the hub shouldn't try and register denoms orignating from the rollapp anyway

[mtsitrin]

why can't the rollapp register a non-native denom to the hub?
the rollapp can't register ibc/ denoms. i.e only native denoms