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

Nitpicks: ICA Audit (InitInterchainAccount Flow) #446

Closed
9 of 12 tasks
seantking opened this issue Sep 29, 2021 · 1 comment
Closed
9 of 12 tasks

Nitpicks: ICA Audit (InitInterchainAccount Flow) #446

seantking opened this issue Sep 29, 2021 · 1 comment
Assignees
@damiannolan
Labels
27-interchain-accounts audit Feedback from implementation audit good first issue Good for newcomers type: code hygiene Clean up code but without changing functionality or interfaces
Milestone
Interchain Accounts

Comments

@seantking
Copy link
Contributor

seantking commented Sep 29, 2021 •
edited by damiannolan
Loading

Summary of Issue

The following nits arose as part of the audit.

  • Remove “for address” in ErrPortAlreadyBound
  • Update error msg for ordered check and counterparty.Port (OnChanOpenInit/OnChanOpenTry)
  • Do basic version string check on OnChanOpenInit/OnChanOpenTry (update ValidateVersion to check full expected version - instead of partial)
  • Update claim capability error msg (wrap err instead of err.Error())
  • RegisterInterchainAccount API rename portID -> counterpartyPortID
  • Wrap parsed Acc check with ErrInvalidVersion instead of using ErrInvalidAccountAddress (OnChanOpenTry)
  • Use activeChannel prefix for range queries (something like -> https://github.com/cosmos/ibc-go/blob/interchain-accounts/modules/apps/27-interchain-accounts/types/keys.go#L41)
  • Update OnChanOpenConfirm godoc
  • Review expected keeper interfaces (double check if we expose anything unnecessarily here)

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@seantking seantking added good first issue Good for newcomers type: code hygiene Clean up code but without changing functionality or interfaces 27-interchain-accounts audit Feedback from implementation audit labels Sep 29, 2021
@seantking seantking added this to the Interchain Accounts milestone Sep 29, 2021
This was referenced Oct 1, 2021
Merged
Merged
@damiannolan damiannolan mentioned this issue Oct 12, 2021
Merged
9 tasks
@damiannolan damiannolan self-assigned this Oct 13, 2021
@seantking
Copy link
Contributor Author

nice @damiannolan 🚅

faddat pushed a commit to notional-labs/ibc-go that referenced this issue Feb 23, 2022
@alpe
Merge pull request cosmos#446 from CosmWasm/send_coins_enabled_414
10a1fef 
Check coin sendable status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@damiannolan damiannolan

Labels
27-interchain-accounts audit Feedback from implementation audit good first issue Good for newcomers type: code hygiene Clean up code but without changing functionality or interfaces
Projects
ibc-go
Archived in project
Milestone
Interchain Accounts
Development

No branches or pull requests
2 participants
@damiannolan @seantking