refactor: simplify IBC redundant relay check in given restructure of SDK v0.46 #1288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of having the function checkRedundancy check if the call is on CheckTx or SimulateTx, only call the function on CheckTx or SimulateTx
colin-axner
requested review from
fedekunze,
AdityaSripal,
crodriguezvega,
seantking,
charleenfei and
damiannolan
as code owners
colin-axner
commented
Apr 25, 2022
| @@ -0,0 +1,129 @@ | |||
| package keeper | |||
There was a problem hiding this comment.
I don't have a strong preference on
modules/core/sdk_middleware/sdk_middleware.go
vs
modules/core/keeper/sdk_middleware.go
but my primary concern is I want to clearly differentiate the difference between IBC middleware and a transaction handler added into the SDK middleware stack
There was a problem hiding this comment.
Prefer this approach as well :)
damiannolan
approved these changes
Apr 25, 2022
crodriguezvega
approved these changes
Apr 25, 2022
crodriguezvega
added a commit
that referenced
this pull request
May 17, 2022
* deps: upgrade of SDK to 0.46 and tendermint to 0.35 * some changes from review comments * some review comments * refactor: simplify IBC redundant relay check in given restructure of SDK v0.46 (#1288) * refactor: simplify ibc redundancy check used as sdk middleware Instead of having the function checkRedundancy check if the call is on CheckTx or SimulateTx, only call the function on CheckTx or SimulateTx * add godoc * more review comments. * another review comment * remove tests since legacy REST endpoints have been removed in SDK 0.46 * chore: update sdk to v0.46.0-beta2 * refactor: ics27 indicator tests of deterministic error codes and message responses (#1349) * begin refactoring ack_test * fix test due to delayed block execution * refactor: switch ics27 response creation to use new SDK version, update tests * fix import * review comment Co-authored-by: colin axnér <[email protected]> * review comment. * pass nil context Co-authored-by: colin axnér <[email protected]> * review comment Co-authored-by: colin axnér <[email protected]> * remove unused import * remove unused import * fix for race condition in tests * remove replace directive to make it build in pre-monterrey mac OS X Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: colin axnér <[email protected]> Co-authored-by: billy rennekamp <[email protected]>
CosmosCar
pushed a commit
to caelus-labs/ibc-go
that referenced
this pull request
Nov 6, 2023
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview Resolves cosmos#1286. <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> ## Checklist <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [x] Visual proof for any user facing features like CLI or documentation updates - [x] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit # Refactor: - Updated the way our system communicates securely with other services. - Moved the import statement for "google.golang.org/grpc" below the import statement for "github.com/ipfs/go-datastore". - Added the import statement for "google.golang.org/grpc/credentials/insecure". - Replaced the "grpc.WithInsecure()" dial option with "grpc.WithTransportCredentials(insecure.NewCredentials())". - Replaced the "grpc.WithInsecure()" option with "grpc.WithTransportCredentials(insecure.NewCredentials())" in the "Start" function. This change enhances the security of our data transmissions, providing a safer environment for user data. Please note, this update is under-the-hood and won't affect the user interface or experience. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
modules/core/sdk_middlewaretomodules/core/keeperIsCheckTx, IsSimulateTxand only calls the tx handler when it is aCheckTxorSimulateTxcloses: #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.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes