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 (TrySendTxFlow) #447

Closed
7 of 13 tasks
seantking opened this issue Sep 29, 2021 · 0 comments
Closed
7 of 13 tasks

Nitpicks: ICA Audit (TrySendTxFlow) #447

seantking opened this issue Sep 29, 2021 · 0 comments
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.

  • TrySendTx (relay.go) channel not found should return for which port id
  • Return nil instead of []byte{} in all returns for keeper/relay.go
  • Require that we pass in an array of sdk.Msg instead of single sdk.Msg (keeper/keeper.go)
  • Add channel/port id in capability not found error (createOutgoingPacket - keeper/relay.go)
  • Add channel/port id in get next send sequence not found error (createOutgoingPacket - keeper/relay.go)
  • Potentially panic on errors that indicate bugs in code? (keeper/keeper.go)
  • ErrUnkownPacketData - ErrUnknownDataType (errors.go)
  • relay.go: AuthenticateTx: Get Interchain address first then loop through signers and return an error if the expected signer is not ICA address
  • relay.go: ExectureTx reduce code by returning when error occurs on executeMsg. Add comment for how cache context is functioning (atomic execution)
  • Fix error type and wrapping in module.go AcknowledgePacket

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 13, 2021
Merged
Merged
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Feb 23, 2022
@ethanfrey
Merge pull request cosmos#447 from CosmWasm/wasmvm_upgrade
bd3cae0 
Upgrade wasmvm to v0.14.0-beta1
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
@colin-axner
add a light client database lock (cosmos#447)
6dcc55b 
Add a lock to prevent multiple processes from attempting to access the light client database at the same time. This typically resulted in unnecessary errors or even panics
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
@colin-axner @akhilkumarpilli
@wonderfan @cryptophonic
refactor update message construction (cosmos#431)
30b7ce5 
* refactor connection handshake update messages

* refactor channel update msg handling

* fix chain id for timeout

* fix build

* refactor state based relaying (fixes update client bug on retries) (cosmos#435)

* bump sdk version to v0.41.3 (cosmos#430)

* bump sdk version

* bump SDK to v0.41.3

* inital work for refactoring state based relaying

* Modify relayPacketFromSequence

* update tendermint client to not prune light blocks (cosmos#437)

* Address comments and fix lint issues

* Fix lint issues

* Remove onRtyErr (lint issue)

* typo fix (cosmos#438)

* disable tm pruning (cosmos#441)

* update release naming (cosmos#442)

* Implement swagger docs and fix path validation (cosmos#434)

* Add swagger setup

* Add some routes docs and swagger ui

* Add few more route docs

* Add swagger docs for remaining routes

* Fix golint issues

* Fix unused lint issues

* check chain-id in AddChain

* add a light client database lock (cosmos#447)

Add a lock to prevent multiple processes from attempting to access the light client database at the same time. This typically resulted in unnecessary errors or even panics

* Close database connection even if second error triggers (cosmos#449)

Co-authored-by: Mark <[email protected]>

* address comments

Co-authored-by: akhilkumarpilli <[email protected]>
Co-authored-by: Afanti <[email protected]>
Co-authored-by: Akhil Kumar P <[email protected]>
Co-authored-by: Mark | Microtick <[email protected]>
Co-authored-by: Mark <[email protected]>

Co-authored-by: akhilkumarpilli <[email protected]>
Co-authored-by: Afanti <[email protected]>
Co-authored-by: Akhil Kumar P <[email protected]>
Co-authored-by: Mark | Microtick <[email protected]>
Co-authored-by: Mark <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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