From aecdf18c007e94e51092009c77e02cbd2bcd4bb7 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 7 Nov 2023 09:44:14 +0100 Subject: [PATCH] [Off-chain] refactor: keyring errors & helpers (#131) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: add `TxClient` interface * chore: add option support to `ReplayObservable` * feat: add `txClient` implementation * test: `txClient` * test: tx client integration * chore: s/tx/transaction/g * chore: update pkg README.md template * wip: client pkg README * docs: fix client pkg godoc comment * refactor: consolidate keyring errors & helpers * refactor: keyring test helpers * fix: flakey test * chore: dial back godoc comments 😅 * chore: revise (and move to godoc.go) `testblock` & `testeventsquery` pkg godoc comment * chore: update go.mod * chore: refactor & condense godoc comments * chore: fix import paths post-update --- internal/testclient/testeventsquery/client.go | 18 +++++++----- internal/testclient/testkeyring/keyring.go | 17 +++++++++++ pkg/client/keyring/errors.go | 19 ++++++++++++ pkg/client/keyring/keyring.go | 29 +++++++++++++++++++ pkg/client/tx/client.go | 17 +++++------ pkg/client/tx/client_integration_test.go | 3 +- pkg/client/tx/client_test.go | 26 +++++++---------- pkg/client/tx/errors.go | 12 -------- 8 files changed, 94 insertions(+), 47 deletions(-) create mode 100644 internal/testclient/testkeyring/keyring.go create mode 100644 pkg/client/keyring/errors.go create mode 100644 pkg/client/keyring/keyring.go diff --git a/internal/testclient/testeventsquery/client.go b/internal/testclient/testeventsquery/client.go index 2c68606ce..fbf7daeb1 100644 --- a/internal/testclient/testeventsquery/client.go +++ b/internal/testclient/testeventsquery/client.go @@ -27,11 +27,12 @@ func NewLocalnetClient(t *testing.T, opts ...client.EventsQueryClientOption) cli } // NewOneTimeEventsQuery creates a mock of the EventsQueryClient which expects -// a single call to the EventsBytes method. query is the query string which is -// expected to be received by that call. -// It returns a mock client whose event bytes method constructs a new observable. -// The caller can simulate blockchain events by sending on the value publishCh -// points to, which is set by this helper function. +// a single call to the EventsBytes method. It returns a mock client whose event +// bytes method always constructs a new observable. query is the query string +// for which event bytes subscription is expected to be for. +// The caller can simulate blockchain events by sending on publishCh, the value +// of which is set to the publish channel of the events bytes observable publish +// channel. func NewOneTimeEventsQuery( ctx context.Context, t *testing.T, @@ -53,11 +54,12 @@ func NewOneTimeEventsQuery( return eventsQueryClient } -// NewOneTimeTxEventsQueryClient creates a mock of the Events that expects +// NewOneTimeTxEventsQueryClient creates a mock of the Events that expects to to // a single call to the EventsBytes method where the query is for transaction // events for sender address matching that of the given key. -// The caller can simulate blockchain events by sending on the value publishCh -// points to, which is set by this helper function. +// The caller can simulate blockchain events by sending on publishCh, the value +// of which is set to the publish channel of the events bytes observable publish +// channel. func NewOneTimeTxEventsQueryClient( ctx context.Context, t *testing.T, diff --git a/internal/testclient/testkeyring/keyring.go b/internal/testclient/testkeyring/keyring.go new file mode 100644 index 000000000..40fbc64c8 --- /dev/null +++ b/internal/testclient/testkeyring/keyring.go @@ -0,0 +1,17 @@ +package testkeyring + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/crypto/keyring" + + "github.com/pokt-network/poktroll/internal/testclient" +) + +// NewTestKeyringWithKey creates a new in-memory keyring with a test key +// with testSigningKeyName as its name. +func NewTestKeyringWithKey(t *testing.T, keyName string) (keyring.Keyring, *keyring.Record) { + keyring := keyring.NewInMemory(testclient.EncodingConfig.Marshaler) + key, _ := testclient.NewKey(t, keyName, keyring) + return keyring, key +} diff --git a/pkg/client/keyring/errors.go b/pkg/client/keyring/errors.go new file mode 100644 index 000000000..7be8a677a --- /dev/null +++ b/pkg/client/keyring/errors.go @@ -0,0 +1,19 @@ +package keyring + +import "cosmossdk.io/errors" + +var ( + // ErrEmptySigningKeyName represents an error which indicates that the + // provided signing key name is empty or unspecified. + ErrEmptySigningKeyName = errors.Register(codespace, 1, "empty signing key name") + + // ErrNoSuchSigningKey represents an error signifying that the requested + // signing key does not exist or could not be located. + ErrNoSuchSigningKey = errors.Register(codespace, 2, "signing key does not exist") + + // ErrSigningKeyAddr is raised when there's a failure in retrieving the + // associated address for the provided signing key. + ErrSigningKeyAddr = errors.Register(codespace, 3, "failed to get address for signing key") + + codespace = "keyring" +) diff --git a/pkg/client/keyring/keyring.go b/pkg/client/keyring/keyring.go new file mode 100644 index 000000000..a77d35b6e --- /dev/null +++ b/pkg/client/keyring/keyring.go @@ -0,0 +1,29 @@ +package keyring + +import ( + cosmoskeyring "github.com/cosmos/cosmos-sdk/crypto/keyring" + cosmostypes "github.com/cosmos/cosmos-sdk/types" +) + +// KeyNameToAddr attempts to retrieve the key with the given name from the +// given keyring and compute its address. +func KeyNameToAddr( + keyName string, + keyring cosmoskeyring.Keyring, +) (cosmostypes.AccAddress, error) { + if keyName == "" { + return nil, ErrEmptySigningKeyName + } + + keyRecord, err := keyring.Key(keyName) + if err != nil { + return nil, ErrNoSuchSigningKey.Wrapf("name %q: %s", keyName, err) + } + + signingAddr, err := keyRecord.GetAddress() + if err != nil { + return nil, ErrSigningKeyAddr.Wrapf("name %q: %s", keyName, err) + } + + return signingAddr, nil +} diff --git a/pkg/client/tx/client.go b/pkg/client/tx/client.go index 805443eb4..1c083559b 100644 --- a/pkg/client/tx/client.go +++ b/pkg/client/tx/client.go @@ -16,6 +16,7 @@ import ( "go.uber.org/multierr" "github.com/pokt-network/poktroll/pkg/client" + "github.com/pokt-network/poktroll/pkg/client/keyring" "github.com/pokt-network/poktroll/pkg/either" "github.com/pokt-network/poktroll/pkg/observable" "github.com/pokt-network/poktroll/pkg/observable/channel" @@ -245,18 +246,14 @@ func (tClient *txClient) SignAndBroadcast( // - ErrSigningKeyAddr if there's an issue retrieving the address for the signing key. // - nil if validation is successful and defaults are set appropriately. func (tClient *txClient) validateConfigAndSetDefaults() error { - if tClient.signingKeyName == "" { - return ErrEmptySigningKeyName - } - - keyRecord, err := tClient.txCtx.GetKeyring().Key(tClient.signingKeyName) - if err != nil { - return ErrNoSuchSigningKey.Wrapf("name %q: %s", tClient.signingKeyName, err) - } - signingAddr, err := keyRecord.GetAddress() + signingAddr, err := keyring.KeyNameToAddr( + tClient.signingKeyName, + tClient.txCtx.GetKeyring(), + ) if err != nil { - return ErrSigningKeyAddr.Wrapf("name %q: %s", tClient.signingKeyName, err) + return err } + tClient.signingAddr = signingAddr if tClient.commitTimeoutHeightOffset <= 0 { diff --git a/pkg/client/tx/client_integration_test.go b/pkg/client/tx/client_integration_test.go index c2d5db6e5..737c8a628 100644 --- a/pkg/client/tx/client_integration_test.go +++ b/pkg/client/tx/client_integration_test.go @@ -10,6 +10,7 @@ import ( "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + "github.com/pokt-network/poktroll/internal/testclient/testkeyring" "github.com/pokt-network/poktroll/pkg/client/tx" "github.com/pokt-network/poktroll/internal/testclient/testblock" @@ -24,7 +25,7 @@ func TestTxClient_SignAndBroadcast_Integration(t *testing.T) { var ctx = context.Background() - keyring, signingKey := newTestKeyringWithKey(t) + keyring, signingKey := testkeyring.NewTestKeyringWithKey(t, testSigningKeyName) eventsQueryClient := testeventsquery.NewLocalnetClient(t) diff --git a/pkg/client/tx/client_test.go b/pkg/client/tx/client_test.go index 6cb5d33da..f6f1d08a9 100644 --- a/pkg/client/tx/client_test.go +++ b/pkg/client/tx/client_test.go @@ -17,8 +17,10 @@ import ( "github.com/pokt-network/poktroll/internal/testclient" "github.com/pokt-network/poktroll/internal/testclient/testblock" "github.com/pokt-network/poktroll/internal/testclient/testeventsquery" + "github.com/pokt-network/poktroll/internal/testclient/testkeyring" "github.com/pokt-network/poktroll/internal/testclient/testtx" "github.com/pokt-network/poktroll/pkg/client" + "github.com/pokt-network/poktroll/pkg/client/keyring" "github.com/pokt-network/poktroll/pkg/client/tx" "github.com/pokt-network/poktroll/pkg/either" apptypes "github.com/pokt-network/poktroll/x/application/types" @@ -49,7 +51,7 @@ func TestTxClient_SignAndBroadcast_Succeeds(t *testing.T) { ctx = context.Background() ) - keyring, signingKey := newTestKeyringWithKey(t) + keyring, signingKey := testkeyring.NewTestKeyringWithKey(t, testSigningKeyName) eventsQueryClient := testeventsquery.NewOneTimeTxEventsQueryClient( ctx, t, signingKey, &eventsBzPublishCh, @@ -118,7 +120,7 @@ func TestTxClient_SignAndBroadcast_Succeeds(t *testing.T) { func TestTxClient_NewTxClient_Error(t *testing.T) { // Construct an empty in-memory keyring. - keyring := cosmoskeyring.NewInMemory(testclient.EncodingConfig.Marshaler) + memKeyring := cosmoskeyring.NewInMemory(testclient.EncodingConfig.Marshaler) tests := []struct { name string @@ -128,12 +130,12 @@ func TestTxClient_NewTxClient_Error(t *testing.T) { { name: "empty signing key name", signingKeyName: "", - expectedErr: tx.ErrEmptySigningKeyName, + expectedErr: keyring.ErrEmptySigningKeyName, }, { name: "signing key does not exist", signingKeyName: "nonexistent", - expectedErr: tx.ErrNoSuchSigningKey, + expectedErr: keyring.ErrNoSuchSigningKey, }, // TODO_TECHDEBT: add coverage for this error case // { @@ -156,7 +158,7 @@ func TestTxClient_NewTxClient_Error(t *testing.T) { eventsQueryClient := mockclient.NewMockEventsQueryClient(ctrl) // Construct a new mock transactions context. - txCtxMock, _ := testtx.NewAnyTimesTxTxContext(t, keyring) + txCtxMock, _ := testtx.NewAnyTimesTxTxContext(t, memKeyring) // Construct a new mock block client. Since we expect the NewTxClient // call to fail, we don't need to set any expectations on this mock. @@ -195,7 +197,7 @@ func TestTxClient_SignAndBroadcast_SyncError(t *testing.T) { ctx = context.Background() ) - keyring, signingKey := newTestKeyringWithKey(t) + keyring, signingKey := testkeyring.NewTestKeyringWithKey(t, testSigningKeyName) // Construct a new mock events query client. Since we expect the // NewTxClient call to fail, we don't need to set any expectations @@ -260,7 +262,7 @@ func TestTxClient_SignAndBroadcast_CheckTxError(t *testing.T) { ctx = context.Background() ) - keyring, signingKey := newTestKeyringWithKey(t) + keyring, signingKey := testkeyring.NewTestKeyringWithKey(t, testSigningKeyName) eventsQueryClient := testeventsquery.NewOneTimeTxEventsQueryClient( ctx, t, signingKey, &eventsBzPublishCh, @@ -323,7 +325,7 @@ func TestTxClient_SignAndBroadcast_Timeout(t *testing.T) { ctx = context.Background() ) - keyring, signingKey := newTestKeyringWithKey(t) + keyring, signingKey := testkeyring.NewTestKeyringWithKey(t, testSigningKeyName) eventsQueryClient := testeventsquery.NewOneTimeTxEventsQueryClient( ctx, t, signingKey, &eventsBzPublishCh, @@ -403,11 +405,3 @@ func TestTxClient_SignAndBroadcast_Timeout(t *testing.T) { func TestTxClient_SignAndBroadcast_MultipleMsgs(t *testing.T) { t.SkipNow() } - -// newTestKeyringWithKey creates a new in-memory keyring with a test key -// with testSigningKeyName as its name. -func newTestKeyringWithKey(t *testing.T) (cosmoskeyring.Keyring, *cosmoskeyring.Record) { - keyring := cosmoskeyring.NewInMemory(testclient.EncodingConfig.Marshaler) - key, _ := testclient.NewKey(t, testSigningKeyName, keyring) - return keyring, key -} diff --git a/pkg/client/tx/errors.go b/pkg/client/tx/errors.go index 474f2ac19..1e43f1d05 100644 --- a/pkg/client/tx/errors.go +++ b/pkg/client/tx/errors.go @@ -3,18 +3,6 @@ package tx import errorsmod "cosmossdk.io/errors" var ( - // ErrEmptySigningKeyName represents an error which indicates that the - // provided signing key name is empty or unspecified. - ErrEmptySigningKeyName = errorsmod.Register(codespace, 1, "empty signing key name") - - // ErrNoSuchSigningKey represents an error signifying that the requested - // signing key does not exist or could not be located. - ErrNoSuchSigningKey = errorsmod.Register(codespace, 2, "signing key does not exist") - - // ErrSigningKeyAddr is raised when there's a failure in retrieving the - // associated address for the provided signing key. - ErrSigningKeyAddr = errorsmod.Register(codespace, 3, "failed to get address for signing key") - // ErrInvalidMsg signifies that there was an issue in validating the // transaction message. This could be due to format, content, or other // constraints imposed on the message.