remove IBC ante handler in favor of using handler #6793
Merged
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
colin-axner
commented
Jul 20, 2020
|
|
||
| attributes := make([]sdk.Attribute, len(msg.GetSigners())) | ||
|
|
||
| for i, signer := range msg.GetSigners() { |
There was a problem hiding this comment.
Is this information useful? If so I can add it to the corresponding handlers
Codecov Report
@@ Coverage Diff @@
## master #6793 +/- ##
==========================================
+ Coverage 60.68% 60.80% +0.12%
==========================================
Files 511 412 -99
Lines 31542 25025 -6517
==========================================
- Hits 19141 15217 -3924
+ Misses 10951 8550 -2401
+ Partials 1450 1258 -192 |
colin-axner
requested review from
aaronc,
alessio,
alexanderbez,
clevinson,
fedekunze and
jgimeno
as code owners
alexanderbez
approved these changes
Jul 20, 2020
fedekunze
approved these changes
Jul 20, 2020
fedekunze
added
the
A:automerge
chengwenxi
pushed a commit
to irisnet/cosmos-sdk
that referenced
this pull request
Jul 22, 2020
* remove IBC ante handler, add calls to handler * move some events upstream to handler * remove change log entry * move ante handler tests to handler * fix build * update docs Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
larry0x
pushed a commit
to larry0x/cosmos-sdk
that referenced
this pull request
May 22, 2023
* remove IBC ante handler, add calls to handler * move some events upstream to handler * remove change log entry * move ante handler tests to handler * fix build * update docs Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
1408eduardo
reviewed
Feb 3, 2024
| @@ -110,16 +110,6 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H | |||
| ), | |||
| ) | |||
|
|
|||
| // localhost client is not updated though messages | |||
| if clientType != exported.Localhost { | |||
1408eduardo
reviewed
Feb 3, 2024
| @@ -4,15 +4,13 @@ import ( | |||
| sdk "github.com/cosmos/cosmos-sdk/types" | |||
| "github.com/cosmos/cosmos-sdk/x/auth/signing" | |||
| "github.com/cosmos/cosmos-sdk/x/auth/types" | |||
| ibcante "github.com/cosmos/cosmos-sdk/x/ibc/ante" | |||
| ibckeeper "github.com/cosmos/cosmos-sdk/x/ibc/keeper" | |||
| ) | |||
|
|
|||
| // NewAnteHandler returns an AnteHandler that checks and increments sequence | |||
| // numbers, checks signatures & account numbers, and deducts fees from the first | |||
| // signer. | |||
| func NewAnteHandler( | |||
1408eduardo
reviewed
Feb 3, 2024
There was a problem hiding this comment.
retire el controlador ante IBC a favor de usar el controlador #6793
Hecho
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
Moves the function calls in the ante handler to the handler level since no state changes occur within the
...Pakcetfunction calls. This design was previously motivated by ADR 15 which was only partially implemented. From what I can tell, the motivation for the existence packet verification and update client within the ante handler was:Intuitive interface for developers - IBC handlers do not need to care about IBC authenticationand possibly to allow update client changes to be persisted despite IBC handler failures, though this is unmentioned in the ADR. The above pro is still applied as IBC application developers still do not handle IBC authentication.
The IBC Keeper and ante handler function are now removed from the
x/authante which I think is a substantial benefit since I do not believe IBC should be a first class citizen in the SDK rather an extra module that the application may choose not to include.The IBC authentication will no longer be susceptible to a griefing attack where an attacker could submit txs that fail in the ante handler without having to pay fees for it. UpdateClient could be a potential vector since it can be computationally expensive. (credit: @AdityaSripal).
The con for this change is
UpdateClientprocessing will not be persisted if any message within a transaction fails. This is negligible and corresponds to existing SDK transaction processing logic. It is also more of a UI issue.ref (undoing some proposed changes): #5230
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