-
Notifications
You must be signed in to change notification settings - Fork 106
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.
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
[CT-1262] add e2e tests for permissioned keys success cases #2479
Changes from 2 commits
04816be
a0af7ea
cad5e81
0b92cbb
3a2487d
906a4ea
7979bef
72a13c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,15 +124,21 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { | |
), | ||
sigVerification: accountplusante.NewCircuitBreakerDecorator( | ||
options.Codec, | ||
accountplusante.NewAuthenticatorDecorator( | ||
options.Codec, | ||
options.AccountplusKeeper, | ||
options.AccountKeeper, | ||
options.SignModeHandler, | ||
sdk.ChainAnteDecorators( | ||
accountplusante.NewAuthenticatorDecorator( | ||
options.Codec, | ||
options.AccountplusKeeper, | ||
options.AccountKeeper, | ||
options.SignModeHandler, | ||
), | ||
), | ||
customante.NewSigVerificationDecorator( | ||
options.AccountKeeper, | ||
options.SignModeHandler, | ||
sdk.ChainAnteDecorators( | ||
ante.NewSetPubKeyDecorator(options.AccountKeeper), | ||
ante.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you remind how how gas consumption is done for accountplus flow? A quick search of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some gas here. we only support secp256k1 so it's a static value. |
||
customante.NewSigVerificationDecorator( | ||
options.AccountKeeper, | ||
options.SignModeHandler, | ||
), | ||
), | ||
), | ||
consumeTxSizeGas: ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), | ||
|
@@ -142,8 +148,6 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { | |
options.FeegrantKeeper, | ||
options.TxFeeChecker, | ||
), | ||
setPubKey: ante.NewSetPubKeyDecorator(options.AccountKeeper), | ||
sigGasConsume: ante.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer), | ||
clobRateLimit: clobante.NewRateLimitDecorator(options.ClobKeeper), | ||
clob: clobante.NewClobDecorator(options.ClobKeeper), | ||
marketUpdates: customante.NewValidateMarketUpdateDecorator( | ||
|
@@ -175,8 +179,6 @@ type lockingAnteHandler struct { | |
sigVerification accountplusante.CircuitBreakerDecorator | ||
consumeTxSizeGas ante.ConsumeTxSizeGasDecorator | ||
deductFee ante.DeductFeeDecorator | ||
setPubKey ante.SetPubKeyDecorator | ||
sigGasConsume ante.SigGasConsumeDecorator | ||
clobRateLimit clobante.ClobRateLimitDecorator | ||
clob clobante.ClobDecorator | ||
marketUpdates customante.ValidateMarketUpdateDecorator | ||
|
@@ -252,15 +254,9 @@ func (h *lockingAnteHandler) clobAnteHandle(ctx sdk.Context, tx sdk.Tx, simulate | |
if ctx, err = h.consumeTxSizeGas.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.setPubKey.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.validateSigCount.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.sigGasConsume.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.replayProtection.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
|
@@ -422,15 +418,9 @@ func (h *lockingAnteHandler) otherMsgAnteHandle(ctx sdk.Context, tx sdk.Tx, simu | |
if ctx, err = h.deductFee.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.setPubKey.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.validateSigCount.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.sigGasConsume.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
if ctx, err = h.replayProtection.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil { | ||
return ctx, err | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,15 +11,15 @@ import ( | |
// the existence of `TxExtension`. | ||
type CircuitBreakerDecorator struct { | ||
cdc codec.BinaryCodec | ||
authenticatorAnteHandlerFlow sdk.AnteDecorator | ||
originalAnteHandlerFlow sdk.AnteDecorator | ||
authenticatorAnteHandlerFlow sdk.AnteHandler | ||
originalAnteHandlerFlow sdk.AnteHandler | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Action Required: Address Remaining The recent changes in
To ensure full consistency and prevent potential integration issues, please update these usages to 🔗 Analysis chainLGTM! Verify impact on codebase. The change from To ensure this change doesn't introduce any issues, please run the following script to check for any remaining usages of 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for remaining usages of sdk.AnteDecorator
# Test: Search for sdk.AnteDecorator usages
rg --type go 'sdk\.AnteDecorator'
Length of output: 10145 |
||
} | ||
|
||
// NewCircuitBreakerDecorator creates a new instance of CircuitBreakerDecorator with the provided parameters. | ||
func NewCircuitBreakerDecorator( | ||
cdc codec.BinaryCodec, | ||
auth sdk.AnteDecorator, | ||
classic sdk.AnteDecorator, | ||
auth sdk.AnteHandler, | ||
jayy04 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
classic sdk.AnteHandler, | ||
) CircuitBreakerDecorator { | ||
return CircuitBreakerDecorator{ | ||
cdc: cdc, | ||
|
@@ -44,9 +44,9 @@ func (ad CircuitBreakerDecorator) AnteHandle( | |
// Check that the authenticator flow is active | ||
if specified, _ := lib.HasSelectedAuthenticatorTxExtensionSpecified(tx, ad.cdc); specified { | ||
// Return and call the AnteHandle function on all the authenticator decorators. | ||
return ad.authenticatorAnteHandlerFlow.AnteHandle(ctx, tx, simulate, next) | ||
return ad.authenticatorAnteHandlerFlow(ctx, tx, simulate) | ||
jayy04 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// Return and call the AnteHandle function on all the original decorators. | ||
return ad.originalAnteHandlerFlow.AnteHandle(ctx, tx, simulate, next) | ||
return ad.originalAnteHandlerFlow(ctx, tx, simulate) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding why was it necessary to move
SetPubKey
andSigGas
under this chain and thus not applicable to smart account flow?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SetPubKey expects the signer of the message to match the public key (link), and I was getting some errors here.
After moving
SetPubKey
, I was getting somepublic key = nil
errors inSigGas
here because public key is not set. So moved this as well.This is also how osmosis does it (link)