From 38329a61a07e323b1c8b986472e3b5903efdfb51 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 30 Nov 2022 10:58:54 +0100 Subject: [PATCH 1/8] refactor: Make SignModeHandler update not API breaking --- CHANGELOG.md | 3 ++- client/tx/tx.go | 19 +++++++++++++-- testutil/sims/tx_helpers.go | 6 ++--- tools/rosetta/converter.go | 10 +++++++- tools/rosetta/go.mod | 9 ++++++-- x/auth/migrations/legacytx/amino_signing.go | 3 +-- .../migrations/legacytx/amino_signing_test.go | 4 ++-- x/auth/signing/handler_map.go | 23 ++++++++++++++++--- x/auth/signing/sign_mode_handler.go | 14 ++++++++++- x/auth/signing/verify.go | 19 +++++++++++++-- x/auth/tx/direct.go | 3 +-- x/auth/tx/direct_aux.go | 3 +-- x/auth/tx/direct_aux_test.go | 10 ++++---- x/auth/tx/direct_test.go | 8 +++---- x/auth/tx/encode_decode_test.go | 2 +- x/auth/tx/legacy_amino_json.go | 3 +-- x/auth/tx/legacy_amino_json_test.go | 8 +++---- x/auth/tx/testutil/suite.go | 5 ++-- 18 files changed, 110 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eda3cafdb03c..20319dd7b845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (signing) [#TODO](https://github.com/cosmos/cosmos-sdk/pull/) Add a new SignModeHandlerWithContext interface with a new GetSignBytesWithContext to get the sign bytes using `context.Context` as an argument to access state. * [13882] (https://github.com/cosmos/cosmos-sdk/pull/13882) Add tx `encode` and `decode` endpoints to amino tx service. > Note: These endpoints encodes and decodes only amino txs. * (config) [#13894](https://github.com/cosmos/cosmos-sdk/pull/13894) Support state streaming configuration in `app.toml` template and default configuration. @@ -177,7 +178,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#13794](https://github.com/cosmos/cosmos-sdk/pull/13794) Most methods on `types/module.AppModule` have been moved to extension interfaces. `module.Manager.Modules` is now of type `map[string]interface{}` to support in parallel the new `cosmossdk.io/core/appmodule.AppModule` API. -* (signing) [#13701](https://github.com/cosmos/cosmos-sdk/pull/) Add `context.Context` as an argument to SignModeHandler's `GetSignBytes` method and `x/auth/signing.VerifySignature`. You can pass `nil` for now, it will only be used once SIGN_MODE_TEXTUAL is live. +* (signing) [#13701](https://github.com/cosmos/cosmos-sdk/pull/) Add `context.Context` as an argument `x/auth/signing.VerifySignature`. * (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Add `GetMinExecutionPeriod` method on DecisionPolicy interface. * (x/auth)[#13780](https://github.com/cosmos/cosmos-sdk/pull/13780) Querying with `id` (type of int64) in `AccountAddressByID` grpc query now throws error, use account-id(type of uint64) instead. * (snapshots) [14048](https://github.com/cosmos/cosmos-sdk/pull/14048) Move the Snapshot package to the store package. This is done in an effort group all storage related logic under one package. diff --git a/client/tx/tx.go b/client/tx/tx.go index b4188aee5cf5..82dc70813ca9 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -158,7 +158,16 @@ func SignWithPrivKey( var sigV2 signing.SignatureV2 // Generate the bytes to be signed. - signBytes, err := txConfig.SignModeHandler().GetSignBytes(ctx, signMode, signerData, txBuilder.GetTx()) + var ( + signBytes []byte + err error + ) + h, ok := txConfig.SignModeHandler().(authsigning.SignModeHandlerWithContext) + if ok { + signBytes, err = h.GetSignBytesWithContext(ctx, signMode, signerData, txBuilder.GetTx()) + } else { + signBytes, err = txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) + } if err != nil { return sigV2, err } @@ -300,7 +309,13 @@ func Sign(ctx context.Context, txf Factory, name string, txBuilder client.TxBuil } // Generate the bytes to be signed. - bytesToSign, err := txf.txConfig.SignModeHandler().GetSignBytes(ctx, signMode, signerData, txBuilder.GetTx()) + var bytesToSign []byte + h, ok := txf.txConfig.SignModeHandler().(authsigning.SignModeHandlerWithContext) + if ok { + bytesToSign, err = h.GetSignBytesWithContext(ctx, signMode, signerData, txBuilder.GetTx()) + } else { + bytesToSign, err = txf.txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) + } if err != nil { return err } diff --git a/testutil/sims/tx_helpers.go b/testutil/sims/tx_helpers.go index 620cc84b3145..73009cd0db2d 100644 --- a/testutil/sims/tx_helpers.go +++ b/testutil/sims/tx_helpers.go @@ -1,7 +1,6 @@ package sims import ( - "context" "math/rand" "testing" "time" @@ -62,8 +61,9 @@ func GenSignedMockTx(r *rand.Rand, txConfig client.TxConfig, msgs []sdk.Msg, fee Sequence: accSeqs[i], PubKey: p.PubKey(), } - // When Textual is wired up, the context argument should be retrieved from the client context. - signBytes, err := txConfig.SignModeHandler().GetSignBytes(context.TODO(), signMode, signerData, tx.GetTx()) + // When Textual is wired up, use GetSignBytesWithContext + // ref: https://github.com/cosmos/cosmos-sdk/issues/13747 + signBytes, err := txConfig.SignModeHandler().GetSignBytes(signMode, signerData, tx.GetTx()) if err != nil { panic(err) } diff --git a/tools/rosetta/converter.go b/tools/rosetta/converter.go index aede4b0d4829..f251c9dd76b1 100644 --- a/tools/rosetta/converter.go +++ b/tools/rosetta/converter.go @@ -115,7 +115,15 @@ func NewConverter(cdc *codec.ProtoCodec, ir codectypes.InterfaceRegistry, cfg sd txDecode: cfg.TxDecoder(), txEncode: cfg.TxEncoder(), bytesToSign: func(tx authsigning.Tx, signerData authsigning.SignerData) (b []byte, err error) { - bytesToSign, err := cfg.SignModeHandler().GetSignBytes(context.TODO(), signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signerData, tx) + var bytesToSign []byte + h, ok := cfg.SignModeHandler().(authsigning.SignModeHandlerWithContext) + if ok { + // Replace context.TODO() with a client side context + // https://github.com/cosmos/cosmos-sdk/issues/13747 + bytesToSign, err = h.GetSignBytesWithContext(context.TODO(), signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signerData, tx) + } else { + bytesToSign, err = cfg.SignModeHandler().GetSignBytes(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signerData, tx) + } if err != nil { return nil, err } diff --git a/tools/rosetta/go.mod b/tools/rosetta/go.mod index beea42160a26..08161801b7fa 100644 --- a/tools/rosetta/go.mod +++ b/tools/rosetta/go.mod @@ -111,5 +111,10 @@ require ( sigs.k8s.io/yaml v1.3.0 // indirect ) -// Update to rosetta-sdk-go temporarly to have `check:spec` passing. See https://github.com/coinbase/rosetta-sdk-go/issues/449 -replace github.com/coinbase/rosetta-sdk-go => github.com/coinbase/rosetta-sdk-go v0.8.2-0.20221007214527-e03849ba430a +replace ( + // temporary until we tag a new sdk version + github.com/cosmos/cosmos-sdk => ../.. + + // Update to rosetta-sdk-go temporarly to have `check:spec` passing. See https://github.com/coinbase/rosetta-sdk-go/issues/449 + github.com/coinbase/rosetta-sdk-go => github.com/coinbase/rosetta-sdk-go v0.8.2-0.20221007214527-e03849ba430a +) diff --git a/x/auth/migrations/legacytx/amino_signing.go b/x/auth/migrations/legacytx/amino_signing.go index 8dd6ec80a10c..4115efa70493 100644 --- a/x/auth/migrations/legacytx/amino_signing.go +++ b/x/auth/migrations/legacytx/amino_signing.go @@ -1,7 +1,6 @@ package legacytx import ( - "context" "fmt" "github.com/cosmos/cosmos-sdk/codec" @@ -33,7 +32,7 @@ func (stdTxSignModeHandler) Modes() []signingtypes.SignMode { } // DefaultMode implements SignModeHandler.GetSignBytes -func (stdTxSignModeHandler) GetSignBytes(_ context.Context, mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { +func (stdTxSignModeHandler) GetSignBytes(mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { if mode != signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON { return nil, fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, mode) } diff --git a/x/auth/migrations/legacytx/amino_signing_test.go b/x/auth/migrations/legacytx/amino_signing_test.go index 0ee548592a25..568ea8b7c271 100644 --- a/x/auth/migrations/legacytx/amino_signing_test.go +++ b/x/auth/migrations/legacytx/amino_signing_test.go @@ -52,7 +52,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { Sequence: seqNum, PubKey: priv1.PubKey(), } - signBz, err := handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) expectedSignBz := StdSignBytes(chainId, accNum, seqNum, timeoutHeight, fee, msgs, memo, nil) @@ -60,7 +60,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { require.Equal(t, expectedSignBz, signBz) // expect error with wrong sign mode - _, err = handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) + _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) require.Error(t, err) } diff --git a/x/auth/signing/handler_map.go b/x/auth/signing/handler_map.go index 494dc3a06bcc..b31237a5d737 100644 --- a/x/auth/signing/handler_map.go +++ b/x/auth/signing/handler_map.go @@ -51,11 +51,28 @@ func (h SignModeHandlerMap) Modes() []signing.SignMode { return h.modes } -// DefaultMode implements SignModeHandler.GetSignBytes -func (h SignModeHandlerMap) GetSignBytes(ctx context.Context, mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) { +// GetSignBytes implements SignModeHandler.GetSignBytes +func (h SignModeHandlerMap) GetSignBytes(mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) { handler, found := h.signModeHandlers[mode] if !found { return nil, fmt.Errorf("can't verify sign mode %s", mode.String()) } - return handler.GetSignBytes(ctx, mode, data, tx) + return handler.GetSignBytes(mode, data, tx) +} + +// GetSignBytesWithContext implements SignModeHandler.GetSignBytesWithContext +func (h SignModeHandlerMap) GetSignBytesWithContext(ctx context.Context, mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) { + handler, found := h.signModeHandlers[mode] + if !found { + return nil, fmt.Errorf("can't verify sign mode %s", mode.String()) + } + + handlerWithContext, ok := handler.(SignModeHandlerWithContext) + if ok { + return handlerWithContext.GetSignBytesWithContext(ctx, mode, data, tx) + } + + // Default to stateless GetSignBytes if the underlying handler does not + // implement WithContext. + return handlerWithContext.GetSignBytes(mode, data, tx) } diff --git a/x/auth/signing/sign_mode_handler.go b/x/auth/signing/sign_mode_handler.go index c0a8a445ba56..4f47d37789ec 100644 --- a/x/auth/signing/sign_mode_handler.go +++ b/x/auth/signing/sign_mode_handler.go @@ -20,7 +20,19 @@ type SignModeHandler interface { // GetSignBytes returns the sign bytes for the provided SignMode, SignerData and Tx, // or an error - GetSignBytes(ctx context.Context, mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) + GetSignBytes(mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) +} + +// SignModeHandlerWithContext is like SignModeHandler, with a new GetSignBytes +// method which takes an additional context.Context argument, to be used to +// access state. Consumers should preferably type-cast to this interface and +// pass in the context.Context arg, and default to SignModeHandler otherwise. +type SignModeHandlerWithContext interface { + SignModeHandler + + // GetSignBytes returns the sign bytes for the provided SignMode, SignerData and Tx, + // or an error + GetSignBytesWithContext(ctx context.Context, mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) } // SignerData is the specific information needed to sign a transaction that generally diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index f5069a309daf..76c7f5336ad4 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -15,7 +15,17 @@ import ( func VerifySignature(ctx context.Context, pubKey cryptotypes.PubKey, signerData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx) error { switch data := sigData.(type) { case *signing.SingleSignatureData: - signBytes, err := handler.GetSignBytes(ctx, data.SignMode, signerData, tx) + var ( + signBytes []byte + err error + ) + handlerWithContext, ok := handler.(SignModeHandlerWithContext) + if ok { + signBytes, err = handlerWithContext.GetSignBytesWithContext(ctx, data.SignMode, signerData, tx) + } else { + signBytes, err = handler.GetSignBytes(data.SignMode, signerData, tx) + } + if err != nil { return err } @@ -30,7 +40,12 @@ func VerifySignature(ctx context.Context, pubKey cryptotypes.PubKey, signerData return fmt.Errorf("expected %T, got %T", (multisig.PubKey)(nil), pubKey) } err := multiPK.VerifyMultisignature(func(mode signing.SignMode) ([]byte, error) { - return handler.GetSignBytes(ctx, mode, signerData, tx) + handlerWithContext, ok := handler.(SignModeHandlerWithContext) + if ok { + return handlerWithContext.GetSignBytesWithContext(ctx, mode, signerData, tx) + } else { + return handler.GetSignBytes(mode, signerData, tx) + } }, data) if err != nil { return err diff --git a/x/auth/tx/direct.go b/x/auth/tx/direct.go index 3b02d8bca95d..4acc52a08dd6 100644 --- a/x/auth/tx/direct.go +++ b/x/auth/tx/direct.go @@ -1,7 +1,6 @@ package tx import ( - "context" "fmt" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" @@ -27,7 +26,7 @@ func (signModeDirectHandler) Modes() []signingtypes.SignMode { } // GetSignBytes implements SignModeHandler.GetSignBytes -func (signModeDirectHandler) GetSignBytes(_ context.Context, mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { +func (signModeDirectHandler) GetSignBytes(mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { if mode != signingtypes.SignMode_SIGN_MODE_DIRECT { return nil, fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_DIRECT, mode) } diff --git a/x/auth/tx/direct_aux.go b/x/auth/tx/direct_aux.go index 37a8d0e757ab..636130981377 100644 --- a/x/auth/tx/direct_aux.go +++ b/x/auth/tx/direct_aux.go @@ -1,7 +1,6 @@ package tx import ( - "context" "fmt" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -29,7 +28,7 @@ func (signModeDirectAuxHandler) Modes() []signingtypes.SignMode { // GetSignBytes implements SignModeHandler.GetSignBytes func (signModeDirectAuxHandler) GetSignBytes( - _ context.Context, mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx, + mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx, ) ([]byte, error) { if mode != signingtypes.SignMode_SIGN_MODE_DIRECT_AUX { return nil, fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, mode) diff --git a/x/auth/tx/direct_aux_test.go b/x/auth/tx/direct_aux_test.go index 08e18c6ce46b..27a5764dbefc 100644 --- a/x/auth/tx/direct_aux_test.go +++ b/x/auth/tx/direct_aux_test.go @@ -80,11 +80,11 @@ func TestDirectAuxHandler(t *testing.T) { modeHandler := signModeDirectAuxHandler{} t.Log("verify fee payer cannot use SIGN_MODE_DIRECT_AUX") - _, err = modeHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, feePayerSigningData, txBuilder.GetTx()) + _, err = modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, feePayerSigningData, txBuilder.GetTx()) require.EqualError(t, err, fmt.Sprintf("fee payer %s cannot sign with %s: unauthorized", feePayerAddr.String(), signingtypes.SignMode_SIGN_MODE_DIRECT_AUX)) t.Log("verify GetSignBytes with generating sign bytes by marshaling signDocDirectAux") - signBytes, err := modeHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, txBuilder.GetTx()) + signBytes, err := modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, txBuilder.GetTx()) require.NoError(t, err) require.NotNil(t, signBytes) @@ -121,7 +121,7 @@ func TestDirectAuxHandler(t *testing.T) { require.NoError(t, err) err = txBuilder.SetSignatures(sig) require.NoError(t, err) - signBytes, err = modeHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, txBuilder.GetTx()) + signBytes, err = modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, txBuilder.GetTx()) require.NoError(t, err) require.Equal(t, expectedSignBytes, signBytes) @@ -148,7 +148,7 @@ func TestDirectAuxModeHandler_nonDIRECT_MODE(t *testing.T) { t.Run(invalidMode.String(), func(t *testing.T) { var dh signModeDirectAuxHandler var signingData signing.SignerData - _, err := dh.GetSignBytes(nil, invalidMode, signingData, nil) + _, err := dh.GetSignBytes(invalidMode, signingData, nil) require.Error(t, err) wantErr := fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, invalidMode) require.Equal(t, err, wantErr) @@ -160,7 +160,7 @@ func TestDirectAuxModeHandler_nonProtoTx(t *testing.T) { var dh signModeDirectAuxHandler var signingData signing.SignerData tx := new(nonProtoTx) - _, err := dh.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, tx) + _, err := dh.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, tx) require.Error(t, err) wantErr := fmt.Errorf("can only handle a protobuf Tx, got %T", tx) require.Equal(t, err, wantErr) diff --git a/x/auth/tx/direct_test.go b/x/auth/tx/direct_test.go index bcc222a9489c..22309639656e 100644 --- a/x/auth/tx/direct_test.go +++ b/x/auth/tx/direct_test.go @@ -75,7 +75,7 @@ func TestDirectModeHandler(t *testing.T) { PubKey: pubkey, } - signBytes, err := modeHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, txBuilder.GetTx()) + signBytes, err := modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, txBuilder.GetTx()) require.NoError(t, err) require.NotNil(t, signBytes) @@ -120,7 +120,7 @@ func TestDirectModeHandler(t *testing.T) { require.NoError(t, err) err = txBuilder.SetSignatures(sig) require.NoError(t, err) - signBytes, err = modeHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, txBuilder.GetTx()) + signBytes, err = modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, txBuilder.GetTx()) require.NoError(t, err) require.Equal(t, expectedSignBytes, signBytes) @@ -141,7 +141,7 @@ func TestDirectModeHandler_nonDIRECT_MODE(t *testing.T) { t.Run(invalidMode.String(), func(t *testing.T) { var dh signModeDirectHandler var signingData signing.SignerData - _, err := dh.GetSignBytes(nil, invalidMode, signingData, nil) + _, err := dh.GetSignBytes(invalidMode, signingData, nil) require.Error(t, err) wantErr := fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_DIRECT, invalidMode) require.Equal(t, err, wantErr) @@ -160,7 +160,7 @@ func TestDirectModeHandler_nonProtoTx(t *testing.T) { var dh signModeDirectHandler var signingData signing.SignerData tx := new(nonProtoTx) - _, err := dh.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) + _, err := dh.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) require.Error(t, err) wantErr := fmt.Errorf("can only handle a protobuf Tx, got %T", tx) require.Equal(t, err, wantErr) diff --git a/x/auth/tx/encode_decode_test.go b/x/auth/tx/encode_decode_test.go index 93caa5115e98..db6c3d995ed9 100644 --- a/x/auth/tx/encode_decode_test.go +++ b/x/auth/tx/encode_decode_test.go @@ -128,7 +128,7 @@ func TestUnknownFields(t *testing.T) { decoder := DefaultTxDecoder(codec.NewProtoCodec(codectypes.NewInterfaceRegistry())) theTx, err := decoder(txBz) require.NoError(t, err) - _, err = handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signing.SignerData{}, theTx) + _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signing.SignerData{}, theTx) require.EqualError(t, err, tt.shouldAminoErr) } }) diff --git a/x/auth/tx/legacy_amino_json.go b/x/auth/tx/legacy_amino_json.go index e0003a5273ce..01274f9f6ed9 100644 --- a/x/auth/tx/legacy_amino_json.go +++ b/x/auth/tx/legacy_amino_json.go @@ -1,7 +1,6 @@ package tx import ( - "context" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" @@ -27,7 +26,7 @@ func (s signModeLegacyAminoJSONHandler) Modes() []signingtypes.SignMode { return []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON} } -func (s signModeLegacyAminoJSONHandler) GetSignBytes(_ context.Context, mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { +func (s signModeLegacyAminoJSONHandler) GetSignBytes(mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { if mode != signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON { return nil, fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, mode) } diff --git a/x/auth/tx/legacy_amino_json_test.go b/x/auth/tx/legacy_amino_json_test.go index f2efb41aa3d6..7df42f92b13d 100644 --- a/x/auth/tx/legacy_amino_json_test.go +++ b/x/auth/tx/legacy_amino_json_test.go @@ -97,7 +97,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { AccountNumber: accNum, Sequence: seqNum, } - signBz, err := handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) require.Equal(t, tc.expectedSignBz, signBz) @@ -116,7 +116,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { } // expect error with wrong sign mode - _, err := handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) + _, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) require.Error(t, err) // expect error with extension options @@ -126,7 +126,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { require.NoError(t, err) bldr.tx.Body.ExtensionOptions = []*cdctypes.Any{any} tx = bldr.GetTx() - _, err = handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.Error(t, err) // expect error with non-critical extension options @@ -134,7 +134,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { buildTx(t, bldr) bldr.tx.Body.NonCriticalExtensionOptions = []*cdctypes.Any{any} tx = bldr.GetTx() - _, err = handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.Error(t, err) } diff --git a/x/auth/tx/testutil/suite.go b/x/auth/tx/testutil/suite.go index 93ec6aa3f76f..7b6ad4673b32 100644 --- a/x/auth/tx/testutil/suite.go +++ b/x/auth/tx/testutil/suite.go @@ -2,7 +2,6 @@ package tx import ( "bytes" - "context" "github.com/stretchr/testify/suite" @@ -135,7 +134,7 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { Sequence: seq1, PubKey: pubkey, } - signBytes, err := signModeHandler.GetSignBytes(context.TODO(), signModeHandler.DefaultMode(), signerData, sigTx) + signBytes, err := signModeHandler.GetSignBytes(signModeHandler.DefaultMode(), signerData, sigTx) s.Require().NoError(err) sigBz, err := privKey.Sign(signBytes) s.Require().NoError(err) @@ -147,7 +146,7 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { Sequence: mseq, PubKey: multisigPk, } - mSignBytes, err := signModeHandler.GetSignBytes(context.TODO(), signModeHandler.DefaultMode(), signerData, sigTx) + mSignBytes, err := signModeHandler.GetSignBytes(signModeHandler.DefaultMode(), signerData, sigTx) s.Require().NoError(err) mSigBz1, err := privKey.Sign(mSignBytes) s.Require().NoError(err) From f50232c5311f7255166826a69139c833449bf989 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 30 Nov 2022 11:01:19 +0100 Subject: [PATCH 2/8] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20319dd7b845..18cf2396734a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,7 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* (signing) [#TODO](https://github.com/cosmos/cosmos-sdk/pull/) Add a new SignModeHandlerWithContext interface with a new GetSignBytesWithContext to get the sign bytes using `context.Context` as an argument to access state. +* (signing) [#14087](https://github.com/cosmos/cosmos-sdk/pull/14087) Add SignModeHandlerWithContext interface with a new `GetSignBytesWithContext` to get the sign bytes using `context.Context` as an argument to access state. * [13882] (https://github.com/cosmos/cosmos-sdk/pull/13882) Add tx `encode` and `decode` endpoints to amino tx service. > Note: These endpoints encodes and decodes only amino txs. * (config) [#13894](https://github.com/cosmos/cosmos-sdk/pull/13894) Support state streaming configuration in `app.toml` template and default configuration. From 6abda7514fe5984b3fe229935502a033357c6503 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 30 Nov 2022 11:16:33 +0100 Subject: [PATCH 3/8] fix tests --- x/auth/signing/handler_map.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/signing/handler_map.go b/x/auth/signing/handler_map.go index b31237a5d737..fcb935c4d586 100644 --- a/x/auth/signing/handler_map.go +++ b/x/auth/signing/handler_map.go @@ -74,5 +74,5 @@ func (h SignModeHandlerMap) GetSignBytesWithContext(ctx context.Context, mode si // Default to stateless GetSignBytes if the underlying handler does not // implement WithContext. - return handlerWithContext.GetSignBytes(mode, data, tx) + return handler.GetSignBytes(mode, data, tx) } From 39e0f6ce2ce6e117d47d71b6e740d1d8bbf02f32 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 30 Nov 2022 11:18:03 +0100 Subject: [PATCH 4/8] add comment --- x/auth/signing/sign_mode_handler.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/auth/signing/sign_mode_handler.go b/x/auth/signing/sign_mode_handler.go index 4f47d37789ec..2c1b85d1b313 100644 --- a/x/auth/signing/sign_mode_handler.go +++ b/x/auth/signing/sign_mode_handler.go @@ -27,6 +27,8 @@ type SignModeHandler interface { // method which takes an additional context.Context argument, to be used to // access state. Consumers should preferably type-cast to this interface and // pass in the context.Context arg, and default to SignModeHandler otherwise. +// This interface is created for backwards compatibility, and will be +// deleted once SDK versions Date: Wed, 30 Nov 2022 11:33:51 +0100 Subject: [PATCH 5/8] Revert rosetta --- tools/rosetta/converter.go | 11 +---------- tools/rosetta/go.mod | 9 ++------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/tools/rosetta/converter.go b/tools/rosetta/converter.go index f251c9dd76b1..4fd1f8366dc9 100644 --- a/tools/rosetta/converter.go +++ b/tools/rosetta/converter.go @@ -2,7 +2,6 @@ package rosetta import ( "bytes" - "context" "encoding/json" "fmt" "reflect" @@ -115,15 +114,7 @@ func NewConverter(cdc *codec.ProtoCodec, ir codectypes.InterfaceRegistry, cfg sd txDecode: cfg.TxDecoder(), txEncode: cfg.TxEncoder(), bytesToSign: func(tx authsigning.Tx, signerData authsigning.SignerData) (b []byte, err error) { - var bytesToSign []byte - h, ok := cfg.SignModeHandler().(authsigning.SignModeHandlerWithContext) - if ok { - // Replace context.TODO() with a client side context - // https://github.com/cosmos/cosmos-sdk/issues/13747 - bytesToSign, err = h.GetSignBytesWithContext(context.TODO(), signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signerData, tx) - } else { - bytesToSign, err = cfg.SignModeHandler().GetSignBytes(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signerData, tx) - } + bytesToSign, err := cfg.SignModeHandler().GetSignBytes(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signerData, tx) if err != nil { return nil, err } diff --git a/tools/rosetta/go.mod b/tools/rosetta/go.mod index 08161801b7fa..beea42160a26 100644 --- a/tools/rosetta/go.mod +++ b/tools/rosetta/go.mod @@ -111,10 +111,5 @@ require ( sigs.k8s.io/yaml v1.3.0 // indirect ) -replace ( - // temporary until we tag a new sdk version - github.com/cosmos/cosmos-sdk => ../.. - - // Update to rosetta-sdk-go temporarly to have `check:spec` passing. See https://github.com/coinbase/rosetta-sdk-go/issues/449 - github.com/coinbase/rosetta-sdk-go => github.com/coinbase/rosetta-sdk-go v0.8.2-0.20221007214527-e03849ba430a -) +// Update to rosetta-sdk-go temporarly to have `check:spec` passing. See https://github.com/coinbase/rosetta-sdk-go/issues/449 +replace github.com/coinbase/rosetta-sdk-go => github.com/coinbase/rosetta-sdk-go v0.8.2-0.20221007214527-e03849ba430a From a1e5c9f843fc9d42dadebf705af0277da47d2bd6 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Thu, 1 Dec 2022 10:09:10 +0100 Subject: [PATCH 6/8] address reviews --- client/tx/tx.go | 19 ++----------------- simapp/go.mod | 3 --- simapp/go.sum | 2 ++ x/auth/signing/verify.go | 25 ++++++++++++++----------- 4 files changed, 18 insertions(+), 31 deletions(-) diff --git a/client/tx/tx.go b/client/tx/tx.go index 82dc70813ca9..79ad9beae73d 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -158,16 +158,7 @@ func SignWithPrivKey( var sigV2 signing.SignatureV2 // Generate the bytes to be signed. - var ( - signBytes []byte - err error - ) - h, ok := txConfig.SignModeHandler().(authsigning.SignModeHandlerWithContext) - if ok { - signBytes, err = h.GetSignBytesWithContext(ctx, signMode, signerData, txBuilder.GetTx()) - } else { - signBytes, err = txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) - } + signBytes, err := authsigning.GetSignBytesWithContext(txConfig.SignModeHandler(), ctx, signMode, signerData, txBuilder.GetTx()) if err != nil { return sigV2, err } @@ -309,13 +300,7 @@ func Sign(ctx context.Context, txf Factory, name string, txBuilder client.TxBuil } // Generate the bytes to be signed. - var bytesToSign []byte - h, ok := txf.txConfig.SignModeHandler().(authsigning.SignModeHandlerWithContext) - if ok { - bytesToSign, err = h.GetSignBytesWithContext(ctx, signMode, signerData, txBuilder.GetTx()) - } else { - bytesToSign, err = txf.txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) - } + bytesToSign, err := authsigning.GetSignBytesWithContext(txf.txConfig.SignModeHandler(), ctx, signMode, signerData, txBuilder.GetTx()) if err != nil { return err } diff --git a/simapp/go.mod b/simapp/go.mod index 77000d832642..9e4e8a281087 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -161,9 +161,6 @@ require ( ) replace ( - // Temporary until we tag a new version - cosmossdk.io/tools/rosetta => ../tools/rosetta - github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0 // Update to rosetta-sdk-go temporarly to have `check:spec` passing. See https://github.com/coinbase/rosetta-sdk-go/issues/449 github.com/coinbase/rosetta-sdk-go => github.com/coinbase/rosetta-sdk-go v0.8.2-0.20221007214527-e03849ba430a diff --git a/simapp/go.sum b/simapp/go.sum index bf49f5227510..2b461133dfad 100644 --- a/simapp/go.sum +++ b/simapp/go.sum @@ -56,6 +56,8 @@ cosmossdk.io/errors v1.0.0-beta.7 h1:gypHW76pTQGVnHKo6QBkb4yFOJjC+sUGRc5Al3Odj1w cosmossdk.io/errors v1.0.0-beta.7/go.mod h1:mz6FQMJRku4bY7aqS/Gwfcmr/ue91roMEKAmDUDpBfE= cosmossdk.io/math v1.0.0-beta.4 h1:JtKedVLGzA0vv84xjYmZ75RKG35Kf2WwcFu8IjRkIIw= cosmossdk.io/math v1.0.0-beta.4/go.mod h1:An0MllWJY6PxibUpnwGk8jOm+a/qIxlKmL5Zyp9NnaM= +cosmossdk.io/tools/rosetta v0.1.0 h1:rJ0sp9bTuGzava+C2b0MFaci/zhINdxSOiJE1FC/UJw= +cosmossdk.io/tools/rosetta v0.1.0/go.mod h1:9wDBVqKC7BDJjk+RWvoE4VXs3Ub/i5rLBrieKpoH+Zw= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0-rc.1 h1:m0VOOB23frXZvAOK44usCgLWvtsxIoMCTBGJZlpmGfU= filippo.io/edwards25519 v1.0.0-rc.1/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index 76c7f5336ad4..7449cafdfff7 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -15,17 +15,7 @@ import ( func VerifySignature(ctx context.Context, pubKey cryptotypes.PubKey, signerData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx) error { switch data := sigData.(type) { case *signing.SingleSignatureData: - var ( - signBytes []byte - err error - ) - handlerWithContext, ok := handler.(SignModeHandlerWithContext) - if ok { - signBytes, err = handlerWithContext.GetSignBytesWithContext(ctx, data.SignMode, signerData, tx) - } else { - signBytes, err = handler.GetSignBytes(data.SignMode, signerData, tx) - } - + signBytes, err := GetSignBytesWithContext(handler, ctx, data.SignMode, signerData, tx) if err != nil { return err } @@ -55,3 +45,16 @@ func VerifySignature(ctx context.Context, pubKey cryptotypes.PubKey, signerData return fmt.Errorf("unexpected SignatureData %T", sigData) } } + +// GetSignBytesWithContext gets the sign bytes from the sign mode handler. It +// checks if the sign mode handler supports SignModeHandlerWithContext, in +// which case it passes the context.Context argument. Otherwise, it fallbacks +// to GetSignBytes. +func GetSignBytesWithContext(h SignModeHandler, ctx context.Context, mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) { + hWithContext, ok := h.(SignModeHandlerWithContext) + if ok { + return hWithContext.GetSignBytesWithContext(ctx, mode, data, tx) + } else { + return h.GetSignBytes(mode, data, tx) + } +} From 75fd8c9cba297eefce3c3d9ba78b2ef87b0be76b Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Thu, 1 Dec 2022 10:11:13 +0100 Subject: [PATCH 7/8] shorter name --- x/auth/signing/verify.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index 7449cafdfff7..0b7ed3fb4eb7 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -51,9 +51,9 @@ func VerifySignature(ctx context.Context, pubKey cryptotypes.PubKey, signerData // which case it passes the context.Context argument. Otherwise, it fallbacks // to GetSignBytes. func GetSignBytesWithContext(h SignModeHandler, ctx context.Context, mode signing.SignMode, data SignerData, tx sdk.Tx) ([]byte, error) { - hWithContext, ok := h.(SignModeHandlerWithContext) + hWithCtx, ok := h.(SignModeHandlerWithContext) if ok { - return hWithContext.GetSignBytesWithContext(ctx, mode, data, tx) + return hWithCtx.GetSignBytesWithContext(ctx, mode, data, tx) } else { return h.GetSignBytes(mode, data, tx) } From 40a256f9b9486ce8290321cb63274d76c7674643 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Thu, 1 Dec 2022 15:52:50 +0100 Subject: [PATCH 8/8] Fix test build --- x/auth/ante/feegrant_test.go | 2 +- x/auth/signing/handler_map_test.go | 6 +++--- x/auth/tx/aux_test.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/auth/ante/feegrant_test.go b/x/auth/ante/feegrant_test.go index 9c75e740ff83..d14b798df1a1 100644 --- a/x/auth/ante/feegrant_test.go +++ b/x/auth/ante/feegrant_test.go @@ -232,7 +232,7 @@ func genTxWithFeeGranter(gen client.TxConfig, msgs []sdk.Msg, feeAmt sdk.Coins, AccountNumber: accNums[i], Sequence: accSeqs[i], } - signBytes, err := gen.SignModeHandler().GetSignBytes(nil, signMode, signerData, tx.GetTx()) + signBytes, err := gen.SignModeHandler().GetSignBytes(signMode, signerData, tx.GetTx()) if err != nil { panic(err) } diff --git a/x/auth/signing/handler_map_test.go b/x/auth/signing/handler_map_test.go index fbe25e26437b..c672ff37aedc 100644 --- a/x/auth/signing/handler_map_test.go +++ b/x/auth/signing/handler_map_test.go @@ -66,16 +66,16 @@ func TestHandlerMap_GetSignBytes(t *testing.T) { Sequence: seqNum, PubKey: priv1.PubKey(), } - signBz, err := handler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) - expectedSignBz, err := aminoJSONHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + expectedSignBz, err := aminoJSONHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) require.Equal(t, expectedSignBz, signBz) // expect error with wrong sign mode - _, err = aminoJSONHandler.GetSignBytes(nil, signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) + _, err = aminoJSONHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) require.Error(t, err) } diff --git a/x/auth/tx/aux_test.go b/x/auth/tx/aux_test.go index 51c37752a978..023b7b477ad7 100644 --- a/x/auth/tx/aux_test.go +++ b/x/auth/tx/aux_test.go @@ -141,7 +141,7 @@ func TestBuilderWithAux(t *testing.T) { Sequence: 15, }) signBz, err = txConfig.SignModeHandler().GetSignBytes( - nil, signing.SignMode_SIGN_MODE_DIRECT, + signing.SignMode_SIGN_MODE_DIRECT, authsigning.SignerData{ Address: feepayerAddr.String(), ChainID: chainID,