Skip to content

Commit

Permalink
address pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jerryfan01234 committed Jul 26, 2024
1 parent 5625b2e commit fe97218
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 14 deletions.
6 changes: 2 additions & 4 deletions protocol/app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@ import (
// struct embedding to include the normal cosmos-sdk `HandlerOptions`.
type HandlerOptions struct {
ante.HandlerOptions
Codec codec.Codec
AuthStoreKey storetypes.StoreKey
// TODO: create interface for accountplus keeper when mocking need arises
// https://github.com/dydxprotocol/v4-chain/pull/1963#discussion_r1691904072
Codec codec.Codec
AuthStoreKey storetypes.StoreKey
AccountplusKeeper *accountpluskeeper.Keeper
ClobKeeper clobtypes.ClobKeeper
PerpetualsKeeper perpetualstypes.PerpetualsKeeper
Expand Down
13 changes: 8 additions & 5 deletions protocol/app/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
"github.com/dydxprotocol/v4-chain/protocol/lib/metrics"
accountpluskeeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
accountplustypes "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/types"
gometrics "github.com/hashicorp/go-metrics"
"google.golang.org/protobuf/types/known/anypb"
)
Expand Down Expand Up @@ -65,9 +66,7 @@ func (svd SigVerificationDecorator) AnteHandle(
}

// Check that signer length and signature length are the same.
// If the lengths are the same, then sigs and signers are ordered correctly. This means that each expected signer
// provided a signature. In cosmos original sigverify, the ordering is explicitly checked.
//https://github.com/cosmos/cosmos-sdk/blob/502450cd1ef0b6bd77807922091893611c370c5d/x/auth/ante/sigverify.go#L189
// The ordering of the sigs and signers have matching ordering (sigs[i] belongs to signers[i]).
if len(sigs) != len(signers) {
err := errorsmod.Wrapf(
sdkerrors.ErrUnauthorized,
Expand Down Expand Up @@ -100,10 +99,14 @@ func (svd SigVerificationDecorator) AnteHandle(
// `GoodTilBlock` for replay protection.
if !skipSequenceValidation {
if accountpluskeeper.IsTimestampNonce(sig.Sequence) {
start := time.Now()
defer telemetry.ModuleMeasureSince(
accountplustypes.ModuleName,
time.Now(),
metrics.TimestampNonce,
metrics.Latency,
)
err = svd.akp.ProcessTimestampNonce(ctx, acc, sig.Sequence)

telemetry.MeasureSince(start, []string{metrics.TimestampNonce, metrics.Latency}...)
if err == nil {
telemetry.IncrCounterWithLabels(
[]string{metrics.TimestampNonce, metrics.Valid, metrics.Count},
Expand Down
6 changes: 1 addition & 5 deletions protocol/x/accountplus/ante/timestampnonce.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,5 @@ func IsTimestampNonceTx(ctx sdk.Context, tx sdk.Tx) (bool, error) {
return false, errorsmod.Wrap(sdkerrors.ErrTxDecode, "more than one signature")
}

if accountpluskeeper.IsTimestampNonce(signatures[0].Sequence) {
return true, nil
} else {
return false, nil
}
return accountpluskeeper.IsTimestampNonce(signatures[0].Sequence), nil
}
1 change: 1 addition & 0 deletions protocol/x/accountplus/keeper/keeper_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
package keeper_test

// TODO: add explicit unit tests for Get and Set funcs
// https://linear.app/dydx/issue/OTE-633/add-explicit-unit-tests-for-get-and-set-methods-for-accountplus-keeper
1 change: 1 addition & 0 deletions protocol/x/accountplus/keeper/timestampnonce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

// Unit tests for ProcessTimestampNonce are not explicitly defined. sigverify_test.go contains test cases that cover
// ProcessTimestampNonce. TODO (low priority): move unit tests from sigverify_test.go to here.
// https://linear.app/dydx/issue/OTE-634/add-explicit-unit-tests-for-accountpluskeeper-processtimestampnonce

func TestIsTimestampNonce(t *testing.T) {
tests := map[string]struct {
Expand Down

0 comments on commit fe97218

Please sign in to comment.