refactor(auth): incorporate set pub key decorator into sig verification decorator #19093
Conversation
| @@ -166,25 +166,6 @@ func TestAnteHandlerSigErrors(t *testing.T) { | |||
| false, | |||
| sdkerrors.ErrUnknownAddress, | |||
| }, | |||
| { | |||
| "save the first account, but second is still unrecognized", | |||
There was a problem hiding this comment.
this test is not valid anymore after the change in behaviours, since the pubkey checking step is done separately during sig verification
| @@ -1121,62 +1102,6 @@ func TestAnteHandlerSetPubKey(t *testing.T) { | |||
| false, | |||
| sdkerrors.ErrWrongSequence, | |||
| }, | |||
| { | |||
| "make sure previous public key has been set after wrong signature", | |||
There was a problem hiding this comment.
this test is not valid anymore because each account has it's own set pub key step
| require.Equal(t, []byte("ok"), okValue) | ||
| } | ||
| // check block gas is always consumed | ||
| baseGas := uint64(54436) // baseGas is the gas consumed before tx msg | ||
| baseGas := uint64(38798) // baseGas is the gas consumed before tx msg |
There was a problem hiding this comment.
we have almost halved the execution costs since we're not redundantly getting and decoding from state, also the account is saved in state only once instead of twice
WalkthroughThe recent updates involve a significant overhaul of the transaction processing, particularly in how public keys are managed and how gas consumption is calculated. The Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| // NOTE: the tx_wrapper implementation returns nil, in case the pubkey is not populated. | ||
| // so we can always expect the pubkey of the signer to be at the same index as the signer | ||
| // itself. If this does not work, it's a failure in the implementation of the interface. | ||
| // we're erroring, but most likely we should be panicking. |
There was a problem hiding this comment.
Why would we panic? Especially in an ante-handler?
There was a problem hiding this comment.
this is a breach of the sdk.Tx interface implementation
There was a problem hiding this comment.
a panic would be caught and treated as an error anyway, so a panic would be futile.
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
…nto tip/auth/refactor_set_pub_key
…on decorator (cosmos#19093) Co-authored-by: unknown unknown <unknown@unknown> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Description
SetPubKeyDecorator was merged into SigVerification, gas consumption is almost halved for a simple tx.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...