IBC Refund to Module Account

[sampocs]

What is the purpose of the change

• The refund function in IBC Transfer's OnTimeoutPacket will panic if trying to refund to a module account
• This is because it tries to send the transfer directly to the module's address, and the module's address is blacklisted in the bank keeper from receiving funds
• The solution is to process the refund using the SendCoinsFromModuleToModule function, rather than SendCoinsFromAccountToModule

Brief Changelog

• Added patches for OnTimeoutPacket and refundPacketToken to the records IBC Module
• Added bank keeper as attribute in records keeper

Testing and Verifying

• Manually verified the change by ...
• Forced a delegation transfer to timeout and verified the following:
  • Before any additional code changes, there was a panic message in hermes and the balance of the module account did not change after the timeout
  • After the code change there was no panic and the balance changed in accordance with the refund
  • Feel free to revert to commit 3e88a9d to test it yourself. Start in local mode, issue a liquid stake and view stride and hermes logs
  • Additionally, to test the refund still succeeds for a non-module address, I added an epochly transfer (hard coded in hooks) directly from one of the validator accounts with a balance log before and after the transfer. Witnessed the validator's balance decrease immediately following the transfer command, yet be replenished come next epoch (because it was refunded).

Documentation and Release Note

• Does this pull request introduce a new feature or user-facing behavior changes? no
• Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
• How is the feature or change documented? TEST-160

[asalzmann]

Nice! Will give a full review in the morning but this looks clean

[asalzmann]

Overall this looks good. I do wonder if we should fork IBC and patch this there - wdyt @sampocs?

[asalzmann]

Per our offline discussion today, we should also try creating a new module account and using that for liquid staking to see if that solves the error. Happy to take a pass at this!

[update]
Chatted with Carlos from the ICF and it sounds like this is the most straightforward way to resolve the issue

[riley-stride]

@sampocs should I review this as it stands or are we going to implement the "new module account" approach?

[asalzmann]

given this fix: cosmos/ibc-go#1907 which will prevent blacklisted addresses from IBC transferring tokens, we should implement the "new module account" approach