From 78f3d33e7da0d21eb663303e08c562d817ceaf5a Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 5 Apr 2024 16:16:20 +0200 Subject: [PATCH 01/13] feat: integrate signer for concurrent tx submission --- go.mod | 2 +- go.sum | 4 +- nodebuilder/module.go | 5 +- nodebuilder/state/core.go | 8 +- nodebuilder/state/keyring.go | 23 ++-- nodebuilder/state/opts.go | 11 +- nodebuilder/testing.go | 19 ++-- nodebuilder/tests/swamp/swamp.go | 5 +- state/core_access.go | 178 ++++++++++++++----------------- state/core_access_test.go | 3 +- state/integration_test.go | 4 +- 11 files changed, 116 insertions(+), 146 deletions(-) diff --git a/go.mod b/go.mod index 48d38c75c4..5db5469cc0 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/BurntSushi/toml v1.3.2 github.com/alecthomas/jsonschema v0.0.0-20220216202328-9eeeec9d044b github.com/benbjohnson/clock v1.3.5 - github.com/celestiaorg/celestia-app v1.7.0 + github.com/celestiaorg/celestia-app v1.8.0-rc0 github.com/celestiaorg/go-fraud v0.2.0 github.com/celestiaorg/go-header v0.6.0 github.com/celestiaorg/go-libp2p-messenger v0.2.0 diff --git a/go.sum b/go.sum index 88e5e60014..12a2e0fab9 100644 --- a/go.sum +++ b/go.sum @@ -358,8 +358,8 @@ github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7 github.com/bwesterb/go-ristretto v1.2.0/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= github.com/c-bata/go-prompt v0.2.2/go.mod h1:VzqtzE2ksDBcdln8G7mk2RX9QyGjH+OVqOCSiVIqS34= github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ= -github.com/celestiaorg/celestia-app v1.7.0 h1:pFIgQzeMD036ulJ2ShUR/3bDGG6kwNAWaLEE0MXHmgg= -github.com/celestiaorg/celestia-app v1.7.0/go.mod h1:z2H47Gs9gYd3GdQ22d5sbcL8/aBMRcVDtUWT64goMaY= +github.com/celestiaorg/celestia-app v1.8.0-rc0 h1:rjEwN0Im1+2QChr8uPfbdomhGL3lEmGlt0cPUodc5JU= +github.com/celestiaorg/celestia-app v1.8.0-rc0/go.mod h1:z2H47Gs9gYd3GdQ22d5sbcL8/aBMRcVDtUWT64goMaY= github.com/celestiaorg/celestia-core v1.35.0-tm-v0.34.29 h1:sXERzNXgyHyqTKNQx4S29C/NMDzgav62DaQDNF49HUQ= github.com/celestiaorg/celestia-core v1.35.0-tm-v0.34.29/go.mod h1:weZR4wYx1Vcw3g1Jc5G8VipG4M+KUDSqeIzyyWszmsQ= github.com/celestiaorg/cosmos-sdk v1.20.1-sdk-v0.46.16 h1:9U9UthIJSOyVjabD5PkD6aczvqlWOyAFTOXw0duPT5k= diff --git a/nodebuilder/module.go b/nodebuilder/module.go index 5bacb50265..aeb587a2f7 100644 --- a/nodebuilder/module.go +++ b/nodebuilder/module.go @@ -28,7 +28,7 @@ func ConstructModule(tp node.Type, network p2p.Network, cfg *Config, store Store if err != nil { return fx.Error(err) } - signer, err := state.KeyringSigner(cfg.State, ks, network) + keyring, keyname, err := state.Keyring(cfg.State, ks) if err != nil { return fx.Error(err) } @@ -45,7 +45,8 @@ func ConstructModule(tp node.Type, network p2p.Network, cfg *Config, store Store fx.Provide(store.Datastore), fx.Provide(store.Keystore), fx.Supply(node.StorePath(store.Path())), - fx.Supply(signer), + fx.Supply(keyring), + fx.Supply(keyname), // modules provided by the node p2p.ConstructModule(tp, &cfg.P2P), state.ConstructModule(tp, &cfg.State, &cfg.Core), diff --git a/nodebuilder/state/core.go b/nodebuilder/state/core.go index 51f2a6bfbf..73a8c33b27 100644 --- a/nodebuilder/state/core.go +++ b/nodebuilder/state/core.go @@ -1,7 +1,8 @@ package state import ( - apptypes "github.com/celestiaorg/celestia-app/x/blob/types" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + libfraud "github.com/celestiaorg/go-fraud" "github.com/celestiaorg/go-header/sync" @@ -16,12 +17,13 @@ import ( // a celestia-core connection. func coreAccessor( corecfg core.Config, - signer *apptypes.KeyringSigner, + keyring keyring.Keyring, + keyname string, sync *sync.Syncer[*header.ExtendedHeader], fraudServ libfraud.Service[*header.ExtendedHeader], opts []state.Option, ) (*state.CoreAccessor, Module, *modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]) { - ca := state.NewCoreAccessor(signer, sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, opts...) + ca := state.NewCoreAccessor(keyring, keyname, sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, opts...) sBreaker := &modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]{ Service: ca, FraudType: byzantine.BadEncoding, diff --git a/nodebuilder/state/keyring.go b/nodebuilder/state/keyring.go index 5aeaff69e2..13b98bf6a4 100644 --- a/nodebuilder/state/keyring.go +++ b/nodebuilder/state/keyring.go @@ -3,26 +3,26 @@ package state import ( kr "github.com/cosmos/cosmos-sdk/crypto/keyring" - apptypes "github.com/celestiaorg/celestia-app/x/blob/types" - "github.com/celestiaorg/celestia-node/libs/keystore" - "github.com/celestiaorg/celestia-node/nodebuilder/p2p" ) const DefaultAccountName = "my_celes_key" -// KeyringSigner constructs a new keyring signer. -// NOTE: we construct keyring signer before constructing node for easier UX +// Keyring constructs a new keyring. +// NOTE: we construct keyring before constructing node for easier UX // as having keyring-backend set to `file` prompts user for password. -func KeyringSigner(cfg Config, ks keystore.Keystore, net p2p.Network) (*apptypes.KeyringSigner, error) { +func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, string, error) { ring := ks.Keyring() + if ring == nil { + panic("keyring is nil") + } var info *kr.Record // if custom keyringAccName provided, find key for that name if cfg.KeyringAccName != "" { keyInfo, err := ring.Key(cfg.KeyringAccName) if err != nil { log.Errorw("failed to find key by given name", "keyring.accname", cfg.KeyringAccName) - return nil, err + return nil, "", err } info = keyInfo } else { @@ -30,15 +30,10 @@ func KeyringSigner(cfg Config, ks keystore.Keystore, net p2p.Network) (*apptypes keyInfo, err := ring.Key(DefaultAccountName) if err != nil { log.Errorw("could not access key in keyring", "name", DefaultAccountName) - return nil, err + return nil, "", err } info = keyInfo } - // construct signer using the default key found / generated above - signer := apptypes.NewKeyringSigner(ring, info.Name, string(net)) - signerInfo := signer.GetSignerInfo() - log.Infow("constructed keyring signer", "backend", cfg.KeyringBackend, "path", ks.Path(), - "key name", signerInfo.Name, "chain-id", string(net)) - return signer, nil + return ring, info.Name, nil } diff --git a/nodebuilder/state/opts.go b/nodebuilder/state/opts.go index 0b357b8396..b359573684 100644 --- a/nodebuilder/state/opts.go +++ b/nodebuilder/state/opts.go @@ -1,13 +1,16 @@ package state import ( + "github.com/cosmos/cosmos-sdk/crypto/keyring" "go.uber.org/fx" - - "github.com/celestiaorg/celestia-app/x/blob/types" ) // WithKeyringSigner overrides the default keyring signer constructed // by the node. -func WithKeyringSigner(signer *types.KeyringSigner) fx.Option { - return fx.Replace(signer) +func WithKeyring(keyring keyring.Keyring) fx.Option { + return fx.Replace(keyring) +} + +func WithKeyName(name string) fx.Option { + return fx.Replace(name) } diff --git a/nodebuilder/testing.go b/nodebuilder/testing.go index c4b3ed4bde..a4cfc94e32 100644 --- a/nodebuilder/testing.go +++ b/nodebuilder/testing.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/fx" - apptypes "github.com/celestiaorg/celestia-app/x/blob/types" libhead "github.com/celestiaorg/go-header" "github.com/celestiaorg/celestia-node/core" @@ -17,9 +16,10 @@ import ( "github.com/celestiaorg/celestia-node/libs/fxutil" "github.com/celestiaorg/celestia-node/nodebuilder/node" "github.com/celestiaorg/celestia-node/nodebuilder/p2p" - "github.com/celestiaorg/celestia-node/nodebuilder/state" ) +const TestKeyringName = "test_celes" + // MockStore provides mock in memory Store for testing purposes. func MockStore(t *testing.T, cfg *Config) Store { t.Helper() @@ -48,10 +48,13 @@ func TestNodeWithConfig(t *testing.T, tp node.Type, cfg *Config, opts ...fx.Opti store := MockStore(t, cfg) ks, err := store.Keystore() require.NoError(t, err) + kr := ks.Keyring() + // create a key in the keystore to be used by the core accessor + _, _, err = kr.NewMnemonic(TestKeyringName, keyring.English, "", "", hd.Secp256k1) + require.NoError(t, err) + cfg.State.KeyringAccName = TestKeyringName opts = append(opts, - // avoid writing keyring on disk - state.WithKeyringSigner(TestKeyringSigner(t, ks.Keyring())), // temp dir for the eds store FIXME: Should be in mem fx.Replace(node.StorePath(t.TempDir())), // avoid requesting trustedPeer during initialization @@ -71,11 +74,3 @@ func TestNodeWithConfig(t *testing.T, tp node.Type, cfg *Config, opts ...fx.Opti require.NoError(t, err) return nd } - -func TestKeyringSigner(t *testing.T, ring keyring.Keyring) *apptypes.KeyringSigner { - signer := apptypes.NewKeyringSigner(ring, "", string(p2p.Private)) - _, _, err := signer.NewMnemonic("my_celes_key", keyring.English, "", - "", hd.Secp256k1) - require.NoError(t, err) - return signer -} diff --git a/nodebuilder/tests/swamp/swamp.go b/nodebuilder/tests/swamp/swamp.go index 9faf69744d..ae72bdd6db 100644 --- a/nodebuilder/tests/swamp/swamp.go +++ b/nodebuilder/tests/swamp/swamp.go @@ -22,7 +22,6 @@ import ( "golang.org/x/exp/maps" "github.com/celestiaorg/celestia-app/test/util/testnode" - apptypes "github.com/celestiaorg/celestia-app/x/blob/types" libhead "github.com/celestiaorg/go-header" "github.com/celestiaorg/celestia-node/core" @@ -258,9 +257,9 @@ func (s *Swamp) NewNodeWithStore( store nodebuilder.Store, options ...fx.Option, ) *nodebuilder.Node { - signer := apptypes.NewKeyringSigner(s.ClientContext.Keyring, s.Accounts[0], s.ClientContext.ChainID) options = append(options, - state.WithKeyringSigner(signer), + state.WithKeyring(s.ClientContext.Keyring), + state.WithKeyName(s.Accounts[0]), ) switch tp { diff --git a/state/core_access.go b/state/core_access.go index 76bfe26d44..2bafc79216 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -11,6 +11,7 @@ import ( sdkErrors "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/api/tendermint/abci" nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node" + "github.com/cosmos/cosmos-sdk/crypto/keyring" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdktypes "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -27,9 +28,10 @@ import ( "google.golang.org/grpc/credentials/insecure" "github.com/celestiaorg/celestia-app/app" + "github.com/celestiaorg/celestia-app/app/encoding" apperrors "github.com/celestiaorg/celestia-app/app/errors" "github.com/celestiaorg/celestia-app/pkg/appconsts" - appblob "github.com/celestiaorg/celestia-app/x/blob" + "github.com/celestiaorg/celestia-app/pkg/user" apptypes "github.com/celestiaorg/celestia-app/x/blob/types" libhead "github.com/celestiaorg/go-header" @@ -66,8 +68,13 @@ type CoreAccessor struct { ctx context.Context cancel context.CancelFunc - signer *apptypes.KeyringSigner - getter libhead.Head[*header.ExtendedHeader] + keyring keyring.Keyring + // TODO: (@cmwaters) - once multiple keys within a signer is supported, + // this will no longer be necessary. + // ref: https://github.com/celestiaorg/celestia-app/issues/3259 + keyname string + signer *user.Signer + getter libhead.Head[*header.ExtendedHeader] stakingCli stakingtypes.QueryClient feeGrantCli feegrant.QueryClient @@ -100,7 +107,8 @@ type CoreAccessor struct { // constructs and returns a new CoreAccessor (state service) with the active // connection. func NewCoreAccessor( - signer *apptypes.KeyringSigner, + keyring keyring.Keyring, + keyname string, getter libhead.Head[*header.ExtendedHeader], coreIP, rpcPort string, @@ -113,7 +121,8 @@ func NewCoreAccessor( prt.RegisterOpDecoder(storetypes.ProofOpSimpleMerkleCommitment, storetypes.CommitmentOpDecoder) ca := &CoreAccessor{ - signer: signer, + keyring: keyring, + keyname: keyname, getter: getter, coreIP: coreIP, rpcPort: rpcPort, @@ -125,7 +134,6 @@ func NewCoreAccessor( opt(ca) } return ca - } func (ca *CoreAccessor) Start(ctx context.Context) error { @@ -157,6 +165,21 @@ func (ca *CoreAccessor) Start(ctx context.Context) error { } ca.rpcCli = cli + encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) + record, err := ca.keyring.Key(ca.keyname) + if err != nil { + return fmt.Errorf("getting key %s: %w", ca.keyname, err) + } + addr, err := record.GetAddress() + if err != nil { + return fmt.Errorf("getting address for key %s: %w", ca.keyname, err) + } + + ca.signer, err = user.SetupSigner(ctx, ca.keyring, ca.coreConn, addr, encCfg) + if err != nil { + return fmt.Errorf("setting up signer: %w", err) + } + ca.minGasPrice, err = ca.queryMinimumGasPrice(ctx) if err != nil { return fmt.Errorf("querying minimum gas price: %w", err) @@ -191,24 +214,6 @@ func (ca *CoreAccessor) cancelCtx() { ca.cancel = nil } -func (ca *CoreAccessor) constructSignedTx( - ctx context.Context, - msg sdktypes.Msg, - opts ...apptypes.TxBuilderOption, -) ([]byte, error) { - // should be called first in order to make a valid tx - err := ca.signer.QueryAccountNumber(ctx, ca.coreConn) - if err != nil { - return nil, err - } - - tx, err := ca.signer.BuildSignedTx(ca.signer.NewTxBuilder(opts...), msg) - if err != nil { - return nil, err - } - return ca.signer.EncodeTx(tx) -} - // SubmitPayForBlob builds, signs, and synchronously submits a MsgPayForBlob. It blocks until the // transaction is committed and returns the TxResponse. If gasLim is set to 0, the method will // automatically estimate the gas limit. If the fee is negative, the method will use the nodes min @@ -267,11 +272,8 @@ func (ca *CoreAccessor) SubmitPayForBlob( if feeGrant != nil { options = append(options, feeGrant) } - response, err := appblob.SubmitPayForBlob( + response, err := ca.signer.SubmitPayForBlob( ctx, - ca.signer, - ca.coreConn, - sdktx.BroadcastMode_BROADCAST_MODE_BLOCK, appblobs, options..., ) @@ -309,19 +311,11 @@ func (ca *CoreAccessor) SubmitPayForBlob( } func (ca *CoreAccessor) AccountAddress(context.Context) (Address, error) { - addr, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return Address{nil}, err - } - return Address{addr}, nil + return Address{ca.signer.Address()}, nil } func (ca *CoreAccessor) Balance(ctx context.Context) (*Balance, error) { - addr, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } - return ca.BalanceForAddress(ctx, Address{addr}) + return ca.BalanceForAddress(ctx, Address{ca.signer.Address()}) } func (ca *CoreAccessor) BalanceForAddress(ctx context.Context, addr Address) (*Balance, error) { @@ -415,17 +409,17 @@ func (ca *CoreAccessor) Transfer( return nil, ErrInvalidAmount } - from, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } coins := sdktypes.NewCoins(sdktypes.NewCoin(app.BondDenom, amount)) - msg := banktypes.NewMsgSend(from, addr, coins) - signedTx, err := ca.constructSignedTx(ctx, msg, apptypes.SetGasLimit(gasLim), withFee(fee)) - if err != nil { - return nil, err + msg := banktypes.NewMsgSend(ca.signer.Address(), addr, coins) + if gasLim == 0 { + var err error + gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + if err != nil { + return nil, fmt.Errorf("estimating gas: %w", err) + } } - return ca.SubmitTx(ctx, signedTx) + + return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) CancelUnbondingDelegation( @@ -440,17 +434,17 @@ func (ca *CoreAccessor) CancelUnbondingDelegation( return nil, ErrInvalidAmount } - from, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgCancelUnbondingDelegation(from, valAddr, height.Int64(), coins) - signedTx, err := ca.constructSignedTx(ctx, msg, apptypes.SetGasLimit(gasLim), withFee(fee)) - if err != nil { - return nil, err + msg := stakingtypes.NewMsgCancelUnbondingDelegation(ca.signer.Address(), valAddr, height.Int64(), coins) + if gasLim == 0 { + var err error + gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + if err != nil { + return nil, fmt.Errorf("estimating gas: %w", err) + } } - return ca.SubmitTx(ctx, signedTx) + + return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) BeginRedelegate( @@ -465,17 +459,17 @@ func (ca *CoreAccessor) BeginRedelegate( return nil, ErrInvalidAmount } - from, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgBeginRedelegate(from, srcValAddr, dstValAddr, coins) - signedTx, err := ca.constructSignedTx(ctx, msg, apptypes.SetGasLimit(gasLim), withFee(fee)) - if err != nil { - return nil, err + msg := stakingtypes.NewMsgBeginRedelegate(ca.signer.Address(), srcValAddr, dstValAddr, coins) + if gasLim == 0 { + var err error + gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + if err != nil { + return nil, fmt.Errorf("estimating gas: %w", err) + } } - return ca.SubmitTx(ctx, signedTx) + + return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) Undelegate( @@ -489,17 +483,16 @@ func (ca *CoreAccessor) Undelegate( return nil, ErrInvalidAmount } - from, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgUndelegate(from, delAddr, coins) - signedTx, err := ca.constructSignedTx(ctx, msg, apptypes.SetGasLimit(gasLim), withFee(fee)) - if err != nil { - return nil, err + msg := stakingtypes.NewMsgUndelegate(ca.signer.Address(), delAddr, coins) + if gasLim == 0 { + var err error + gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + if err != nil { + return nil, fmt.Errorf("estimating gas: %w", err) + } } - return ca.SubmitTx(ctx, signedTx) + return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) Delegate( @@ -513,27 +506,23 @@ func (ca *CoreAccessor) Delegate( return nil, ErrInvalidAmount } - from, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgDelegate(from, delAddr, coins) - signedTx, err := ca.constructSignedTx(ctx, msg, apptypes.SetGasLimit(gasLim), withFee(fee)) - if err != nil { - return nil, err + msg := stakingtypes.NewMsgDelegate(ca.signer.Address(), delAddr, coins) + if gasLim == 0 { + var err error + gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + if err != nil { + return nil, fmt.Errorf("estimating gas: %w", err) + } } - return ca.SubmitTx(ctx, signedTx) + return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) QueryDelegation( ctx context.Context, valAddr ValAddress, ) (*stakingtypes.QueryDelegationResponse, error) { - delAddr, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } + delAddr := ca.signer.Address() return ca.stakingCli.Delegation(ctx, &stakingtypes.QueryDelegationRequest{ DelegatorAddr: delAddr.String(), ValidatorAddr: valAddr.String(), @@ -544,10 +533,7 @@ func (ca *CoreAccessor) QueryUnbonding( ctx context.Context, valAddr ValAddress, ) (*stakingtypes.QueryUnbondingDelegationResponse, error) { - delAddr, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } + delAddr := ca.signer.Address() return ca.stakingCli.UnbondingDelegation(ctx, &stakingtypes.QueryUnbondingDelegationRequest{ DelegatorAddr: delAddr.String(), ValidatorAddr: valAddr.String(), @@ -558,10 +544,7 @@ func (ca *CoreAccessor) QueryRedelegations( srcValAddr, dstValAddr ValAddress, ) (*stakingtypes.QueryRedelegationsResponse, error) { - delAddr, err := ca.signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } + delAddr := ca.signer.Address() return ca.stakingCli.Redelegations(ctx, &stakingtypes.QueryRedelegationsRequest{ DelegatorAddr: delAddr.String(), SrcValidatorAddr: srcValAddr.String(), @@ -665,8 +648,3 @@ func (ca *CoreAccessor) queryMinimumGasPrice( } return coins.AmountOf(app.BondDenom).MustFloat64(), nil } - -func withFee(fee Int) apptypes.TxBuilderOption { - gasFee := sdktypes.NewCoins(sdktypes.NewCoin(app.BondDenom, fee)) - return apptypes.SetFeeAmount(gasFee) -} diff --git a/state/core_access_test.go b/state/core_access_test.go index ad7b916ea3..7b091b7575 100644 --- a/state/core_access_test.go +++ b/state/core_access_test.go @@ -36,8 +36,7 @@ func TestSubmitPayForBlob(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - signer := blobtypes.NewKeyringSigner(cctx.Keyring, accounts[0], cctx.ChainID) - ca := NewCoreAccessor(signer, nil, "127.0.0.1", extractPort(rpcAddr), extractPort(grpcAddr)) + ca := NewCoreAccessor(cctx.Keyring, accounts[0], nil, "127.0.0.1", extractPort(rpcAddr), extractPort(grpcAddr)) // start the accessor err := ca.Start(ctx) require.NoError(t, err) diff --git a/state/integration_test.go b/state/integration_test.go index 1aa342649c..eba1fa69bd 100644 --- a/state/integration_test.go +++ b/state/integration_test.go @@ -18,7 +18,6 @@ import ( "github.com/celestiaorg/celestia-app/app" "github.com/celestiaorg/celestia-app/test/util/testfactory" "github.com/celestiaorg/celestia-app/test/util/testnode" - blobtypes "github.com/celestiaorg/celestia-app/x/blob/types" libhead "github.com/celestiaorg/go-header" "github.com/celestiaorg/celestia-node/core" @@ -49,8 +48,7 @@ func (s *IntegrationTestSuite) SetupSuite() { s.cctx = core.StartTestNodeWithConfig(s.T(), cfg) s.accounts = cfg.Accounts - signer := blobtypes.NewKeyringSigner(s.cctx.Keyring, s.accounts[0], s.cctx.ChainID) - accessor := NewCoreAccessor(signer, localHeader{s.cctx.Client}, "", "", "") + accessor := NewCoreAccessor(s.cctx.Keyring, s.accounts[0], localHeader{s.cctx.Client}, "", "", "") setClients(accessor, s.cctx.GRPCClient, s.cctx.Client) s.accessor = accessor From 08baa9319684be4bd8c5dd84da69abd9a48f1984 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 5 Apr 2024 16:25:17 +0200 Subject: [PATCH 02/13] remove panic --- nodebuilder/state/keyring.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/nodebuilder/state/keyring.go b/nodebuilder/state/keyring.go index 13b98bf6a4..0b488b1b7d 100644 --- a/nodebuilder/state/keyring.go +++ b/nodebuilder/state/keyring.go @@ -13,9 +13,6 @@ const DefaultAccountName = "my_celes_key" // as having keyring-backend set to `file` prompts user for password. func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, string, error) { ring := ks.Keyring() - if ring == nil { - panic("keyring is nil") - } var info *kr.Record // if custom keyringAccName provided, find key for that name if cfg.KeyringAccName != "" { From 9881da9ba18d2b47632057a08d1ee7a82e8a22a6 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Mon, 8 Apr 2024 13:20:24 +0200 Subject: [PATCH 03/13] fix(nodebuilder): refactor state module construction --- nodebuilder/module.go | 7 +------ nodebuilder/state/keyring.go | 6 ++++-- nodebuilder/state/module.go | 5 +++++ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/nodebuilder/module.go b/nodebuilder/module.go index aeb587a2f7..8f196f3b1d 100644 --- a/nodebuilder/module.go +++ b/nodebuilder/module.go @@ -28,14 +28,11 @@ func ConstructModule(tp node.Type, network p2p.Network, cfg *Config, store Store if err != nil { return fx.Error(err) } - keyring, keyname, err := state.Keyring(cfg.State, ks) - if err != nil { - return fx.Error(err) - } baseComponents := fx.Options( fx.Supply(tp), fx.Supply(network), + fx.Supply(ks), fx.Provide(p2p.BootstrappersFor), fx.Provide(func(lc fx.Lifecycle) context.Context { return fxutil.WithLifecycle(context.Background(), lc) @@ -45,8 +42,6 @@ func ConstructModule(tp node.Type, network p2p.Network, cfg *Config, store Store fx.Provide(store.Datastore), fx.Provide(store.Keystore), fx.Supply(node.StorePath(store.Path())), - fx.Supply(keyring), - fx.Supply(keyname), // modules provided by the node p2p.ConstructModule(tp, &cfg.P2P), state.ConstructModule(tp, &cfg.State, &cfg.Core), diff --git a/nodebuilder/state/keyring.go b/nodebuilder/state/keyring.go index 0b488b1b7d..25723ef5b7 100644 --- a/nodebuilder/state/keyring.go +++ b/nodebuilder/state/keyring.go @@ -8,10 +8,12 @@ import ( const DefaultAccountName = "my_celes_key" +type AccName string + // Keyring constructs a new keyring. // NOTE: we construct keyring before constructing node for easier UX // as having keyring-backend set to `file` prompts user for password. -func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, string, error) { +func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, AccName, error) { ring := ks.Keyring() var info *kr.Record // if custom keyringAccName provided, find key for that name @@ -32,5 +34,5 @@ func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, string, error) { info = keyInfo } - return ring, info.Name, nil + return ring, AccName(info.Name), nil } diff --git a/nodebuilder/state/module.go b/nodebuilder/state/module.go index e8d701de75..116a24ceda 100644 --- a/nodebuilder/state/module.go +++ b/nodebuilder/state/module.go @@ -3,11 +3,13 @@ package state import ( "context" + "github.com/cosmos/cosmos-sdk/crypto/keyring" logging "github.com/ipfs/go-log/v2" "go.uber.org/fx" "github.com/celestiaorg/celestia-node/header" "github.com/celestiaorg/celestia-node/libs/fxutil" + "github.com/celestiaorg/celestia-node/libs/keystore" "github.com/celestiaorg/celestia-node/nodebuilder/core" modfraud "github.com/celestiaorg/celestia-node/nodebuilder/fraud" "github.com/celestiaorg/celestia-node/nodebuilder/node" @@ -29,6 +31,9 @@ func ConstructModule(tp node.Type, cfg *Config, coreCfg *core.Config) fx.Option fx.Supply(*cfg), fx.Error(cfgErr), fx.Supply(opts), + fx.Provide(func(ks keystore.Keystore) (keyring.Keyring, AccName, error) { + return Keyring(*cfg, ks) + }), fxutil.ProvideIf(coreCfg.IsEndpointConfigured(), fx.Annotate( coreAccessor, fx.OnStart(func(ctx context.Context, From 6d3eccd5beb5ee687acd29cd8146980fccd432de Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:45:34 +0200 Subject: [PATCH 04/13] fix(state): ensure signer setup is non-blocking for node startup --- state/core_access.go | 126 ++++++++++++++++++++++++++------------ state/core_access_test.go | 6 +- state/integration_test.go | 5 +- 3 files changed, 94 insertions(+), 43 deletions(-) diff --git a/state/core_access.go b/state/core_access.go index 2bafc79216..5108c2225e 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -72,9 +72,10 @@ type CoreAccessor struct { // TODO: (@cmwaters) - once multiple keys within a signer is supported, // this will no longer be necessary. // ref: https://github.com/celestiaorg/celestia-app/issues/3259 - keyname string - signer *user.Signer - getter libhead.Head[*header.ExtendedHeader] + signer *user.Signer + addr sdktypes.AccAddress + + getter libhead.Head[*header.ExtendedHeader] stakingCli stakingtypes.QueryClient feeGrantCli feegrant.QueryClient @@ -114,15 +115,24 @@ func NewCoreAccessor( rpcPort string, grpcPort string, options ...Option, -) *CoreAccessor { +) (*CoreAccessor, error) { // create verifier prt := merkle.DefaultProofRuntime() prt.RegisterOpDecoder(storetypes.ProofOpIAVLCommitment, storetypes.CommitmentOpDecoder) prt.RegisterOpDecoder(storetypes.ProofOpSimpleMerkleCommitment, storetypes.CommitmentOpDecoder) + record, err := keyring.Key(keyname) + if err != nil { + return nil, fmt.Errorf("getting key %s: %w", keyname, err) + } + addr, err := record.GetAddress() + if err != nil { + return nil, fmt.Errorf("getting address for key %s: %w", keyname, err) + } + ca := &CoreAccessor{ keyring: keyring, - keyname: keyname, + addr: addr, getter: getter, coreIP: coreIP, rpcPort: rpcPort, @@ -133,7 +143,7 @@ func NewCoreAccessor( for _, opt := range options { opt(ca) } - return ca + return ca, nil } func (ca *CoreAccessor) Start(ctx context.Context) error { @@ -165,19 +175,9 @@ func (ca *CoreAccessor) Start(ctx context.Context) error { } ca.rpcCli = cli - encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) - record, err := ca.keyring.Key(ca.keyname) - if err != nil { - return fmt.Errorf("getting key %s: %w", ca.keyname, err) - } - addr, err := record.GetAddress() - if err != nil { - return fmt.Errorf("getting address for key %s: %w", ca.keyname, err) - } - - ca.signer, err = user.SetupSigner(ctx, ca.keyring, ca.coreConn, addr, encCfg) + err = ca.setupSigner(ctx) if err != nil { - return fmt.Errorf("setting up signer: %w", err) + log.Warnw("failed to set up signer, check if node's account is funded", "err", err) } ca.minGasPrice, err = ca.queryMinimumGasPrice(ctx) @@ -224,6 +224,11 @@ func (ca *CoreAccessor) SubmitPayForBlob( gasLim uint64, blobs []*blob.Blob, ) (*TxResponse, error) { + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + if len(blobs) == 0 { return nil, errors.New("state: no blobs provided") } @@ -272,7 +277,7 @@ func (ca *CoreAccessor) SubmitPayForBlob( if feeGrant != nil { options = append(options, feeGrant) } - response, err := ca.signer.SubmitPayForBlob( + response, err := signer.SubmitPayForBlob( ctx, appblobs, options..., @@ -311,11 +316,11 @@ func (ca *CoreAccessor) SubmitPayForBlob( } func (ca *CoreAccessor) AccountAddress(context.Context) (Address, error) { - return Address{ca.signer.Address()}, nil + return Address{ca.addr}, nil } func (ca *CoreAccessor) Balance(ctx context.Context) (*Balance, error) { - return ca.BalanceForAddress(ctx, Address{ca.signer.Address()}) + return ca.BalanceForAddress(ctx, Address{ca.addr}) } func (ca *CoreAccessor) BalanceForAddress(ctx context.Context, addr Address) (*Balance, error) { @@ -405,21 +410,26 @@ func (ca *CoreAccessor) Transfer( fee Int, gasLim uint64, ) (*TxResponse, error) { + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + if amount.IsNil() || amount.Int64() <= 0 { return nil, ErrInvalidAmount } coins := sdktypes.NewCoins(sdktypes.NewCoin(app.BondDenom, amount)) - msg := banktypes.NewMsgSend(ca.signer.Address(), addr, coins) + msg := banktypes.NewMsgSend(signer.Address(), addr, coins) if gasLim == 0 { var err error - gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + gasLim, err = signer.EstimateGas(ctx, []sdktypes.Msg{msg}) if err != nil { return nil, fmt.Errorf("estimating gas: %w", err) } } - return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) + return signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) CancelUnbondingDelegation( @@ -430,21 +440,26 @@ func (ca *CoreAccessor) CancelUnbondingDelegation( fee Int, gasLim uint64, ) (*TxResponse, error) { + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + if amount.IsNil() || amount.Int64() <= 0 { return nil, ErrInvalidAmount } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgCancelUnbondingDelegation(ca.signer.Address(), valAddr, height.Int64(), coins) + msg := stakingtypes.NewMsgCancelUnbondingDelegation(signer.Address(), valAddr, height.Int64(), coins) if gasLim == 0 { var err error - gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + gasLim, err = signer.EstimateGas(ctx, []sdktypes.Msg{msg}) if err != nil { return nil, fmt.Errorf("estimating gas: %w", err) } } - return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) + return signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) BeginRedelegate( @@ -455,21 +470,26 @@ func (ca *CoreAccessor) BeginRedelegate( fee Int, gasLim uint64, ) (*TxResponse, error) { + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + if amount.IsNil() || amount.Int64() <= 0 { return nil, ErrInvalidAmount } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgBeginRedelegate(ca.signer.Address(), srcValAddr, dstValAddr, coins) + msg := stakingtypes.NewMsgBeginRedelegate(signer.Address(), srcValAddr, dstValAddr, coins) if gasLim == 0 { var err error - gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + gasLim, err = signer.EstimateGas(ctx, []sdktypes.Msg{msg}) if err != nil { return nil, fmt.Errorf("estimating gas: %w", err) } } - return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) + return signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) Undelegate( @@ -479,20 +499,25 @@ func (ca *CoreAccessor) Undelegate( fee Int, gasLim uint64, ) (*TxResponse, error) { + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + if amount.IsNil() || amount.Int64() <= 0 { return nil, ErrInvalidAmount } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgUndelegate(ca.signer.Address(), delAddr, coins) + msg := stakingtypes.NewMsgUndelegate(signer.Address(), delAddr, coins) if gasLim == 0 { var err error - gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + gasLim, err = signer.EstimateGas(ctx, []sdktypes.Msg{msg}) if err != nil { return nil, fmt.Errorf("estimating gas: %w", err) } } - return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) + return signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) Delegate( @@ -502,27 +527,32 @@ func (ca *CoreAccessor) Delegate( fee Int, gasLim uint64, ) (*TxResponse, error) { + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + if amount.IsNil() || amount.Int64() <= 0 { return nil, ErrInvalidAmount } coins := sdktypes.NewCoin(app.BondDenom, amount) - msg := stakingtypes.NewMsgDelegate(ca.signer.Address(), delAddr, coins) + msg := stakingtypes.NewMsgDelegate(signer.Address(), delAddr, coins) if gasLim == 0 { var err error - gasLim, err = ca.signer.EstimateGas(ctx, []sdktypes.Msg{msg}) + gasLim, err = signer.EstimateGas(ctx, []sdktypes.Msg{msg}) if err != nil { return nil, fmt.Errorf("estimating gas: %w", err) } } - return ca.signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) + return signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), user.SetFee(fee.Uint64())) } func (ca *CoreAccessor) QueryDelegation( ctx context.Context, valAddr ValAddress, ) (*stakingtypes.QueryDelegationResponse, error) { - delAddr := ca.signer.Address() + delAddr := ca.addr return ca.stakingCli.Delegation(ctx, &stakingtypes.QueryDelegationRequest{ DelegatorAddr: delAddr.String(), ValidatorAddr: valAddr.String(), @@ -533,7 +563,7 @@ func (ca *CoreAccessor) QueryUnbonding( ctx context.Context, valAddr ValAddress, ) (*stakingtypes.QueryUnbondingDelegationResponse, error) { - delAddr := ca.signer.Address() + delAddr := ca.addr return ca.stakingCli.UnbondingDelegation(ctx, &stakingtypes.QueryUnbondingDelegationRequest{ DelegatorAddr: delAddr.String(), ValidatorAddr: valAddr.String(), @@ -544,7 +574,7 @@ func (ca *CoreAccessor) QueryRedelegations( srcValAddr, dstValAddr ValAddress, ) (*stakingtypes.QueryRedelegationsResponse, error) { - delAddr := ca.signer.Address() + delAddr := ca.addr return ca.stakingCli.Redelegations(ctx, &stakingtypes.QueryRedelegationsRequest{ DelegatorAddr: delAddr.String(), SrcValidatorAddr: srcValAddr.String(), @@ -648,3 +678,21 @@ func (ca *CoreAccessor) queryMinimumGasPrice( } return coins.AmountOf(app.BondDenom).MustFloat64(), nil } + +func (ca *CoreAccessor) getSigner(ctx context.Context) (*user.Signer, error) { + if ca.signer == nil { + err := ca.setupSigner(ctx) + if err != nil { + return nil, err + } + } + return ca.signer, nil +} + +func (ca *CoreAccessor) setupSigner(ctx context.Context) error { + encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) + + var err error + ca.signer, err = user.SetupSigner(ctx, ca.keyring, ca.coreConn, ca.addr, encCfg) + return err +} diff --git a/state/core_access_test.go b/state/core_access_test.go index 7b091b7575..6e56c896f0 100644 --- a/state/core_access_test.go +++ b/state/core_access_test.go @@ -36,9 +36,11 @@ func TestSubmitPayForBlob(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - ca := NewCoreAccessor(cctx.Keyring, accounts[0], nil, "127.0.0.1", extractPort(rpcAddr), extractPort(grpcAddr)) + ca, err := NewCoreAccessor(cctx.Keyring, accounts[0], nil, "127.0.0.1", extractPort(rpcAddr), + extractPort(grpcAddr)) + require.NoError(t, err) // start the accessor - err := ca.Start(ctx) + err = ca.Start(ctx) require.NoError(t, err) t.Cleanup(func() { _ = ca.Stop(ctx) diff --git a/state/integration_test.go b/state/integration_test.go index eba1fa69bd..6d2a38c8a7 100644 --- a/state/integration_test.go +++ b/state/integration_test.go @@ -48,12 +48,13 @@ func (s *IntegrationTestSuite) SetupSuite() { s.cctx = core.StartTestNodeWithConfig(s.T(), cfg) s.accounts = cfg.Accounts - accessor := NewCoreAccessor(s.cctx.Keyring, s.accounts[0], localHeader{s.cctx.Client}, "", "", "") + accessor, err := NewCoreAccessor(s.cctx.Keyring, s.accounts[0], localHeader{s.cctx.Client}, "", "", "") + require.NoError(s.T(), err) setClients(accessor, s.cctx.GRPCClient, s.cctx.Client) s.accessor = accessor // required to ensure the Head request is non-nil - _, err := s.cctx.WaitForHeight(3) + _, err = s.cctx.WaitForHeight(3) require.NoError(s.T(), err) } From 58419bf07ccee117d858297a67d8f4d19fb65de0 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:33:41 +0200 Subject: [PATCH 05/13] fix(nodebuilder/tests): Fix replace keyring in swamp --- nodebuilder/state/opts.go | 12 +++++++----- nodebuilder/tests/blob_test.go | 2 -- nodebuilder/tests/swamp/swamp.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/nodebuilder/state/opts.go b/nodebuilder/state/opts.go index b359573684..e74d85bece 100644 --- a/nodebuilder/state/opts.go +++ b/nodebuilder/state/opts.go @@ -1,16 +1,18 @@ package state import ( - "github.com/cosmos/cosmos-sdk/crypto/keyring" + kr "github.com/cosmos/cosmos-sdk/crypto/keyring" "go.uber.org/fx" + + "github.com/celestiaorg/celestia-node/libs/fxutil" ) -// WithKeyringSigner overrides the default keyring signer constructed +// WithKeyring overrides the default keyring constructed // by the node. -func WithKeyring(keyring keyring.Keyring) fx.Option { - return fx.Replace(keyring) +func WithKeyring(keyring kr.Keyring) fx.Option { + return fxutil.ReplaceAs(keyring, new(kr.Keyring)) } -func WithKeyName(name string) fx.Option { +func WithKeyName(name AccName) fx.Option { return fx.Replace(name) } diff --git a/nodebuilder/tests/blob_test.go b/nodebuilder/tests/blob_test.go index 7eb225a14a..9b6496ab9b 100644 --- a/nodebuilder/tests/blob_test.go +++ b/nodebuilder/tests/blob_test.go @@ -37,10 +37,8 @@ func TestBlobModule(t *testing.T) { blobs = append(blobs, blob) } - require.NoError(t, err) bridge := sw.NewBridgeNode() require.NoError(t, bridge.Start(ctx)) - addrs, err := peer.AddrInfoToP2pAddrs(host.InfoFromHost(bridge.Host)) require.NoError(t, err) diff --git a/nodebuilder/tests/swamp/swamp.go b/nodebuilder/tests/swamp/swamp.go index ae72bdd6db..42fc0e4062 100644 --- a/nodebuilder/tests/swamp/swamp.go +++ b/nodebuilder/tests/swamp/swamp.go @@ -259,7 +259,7 @@ func (s *Swamp) NewNodeWithStore( ) *nodebuilder.Node { options = append(options, state.WithKeyring(s.ClientContext.Keyring), - state.WithKeyName(s.Accounts[0]), + state.WithKeyName(state.AccName(s.Accounts[0])), ) switch tp { From f1c288a380880e7d42bb7c4ea0010b99db88e0c4 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:45:10 +0200 Subject: [PATCH 06/13] doc(nodebuilder/state): doc WithKeyName --- nodebuilder/state/opts.go | 1 + 1 file changed, 1 insertion(+) diff --git a/nodebuilder/state/opts.go b/nodebuilder/state/opts.go index e74d85bece..54a3530be9 100644 --- a/nodebuilder/state/opts.go +++ b/nodebuilder/state/opts.go @@ -13,6 +13,7 @@ func WithKeyring(keyring kr.Keyring) fx.Option { return fxutil.ReplaceAs(keyring, new(kr.Keyring)) } +// WithKeyName configures the signer to use the given key. func WithKeyName(name AccName) fx.Option { return fx.Replace(name) } From 78c8c223a3cf2bcb8f0f35145f3df6405a11eed1 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:44:46 +0200 Subject: [PATCH 07/13] prog --- nodebuilder/state/core.go | 15 ++++++++++--- state/core_access.go | 47 ++++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/nodebuilder/state/core.go b/nodebuilder/state/core.go index 73a8c33b27..c31c338485 100644 --- a/nodebuilder/state/core.go +++ b/nodebuilder/state/core.go @@ -22,12 +22,21 @@ func coreAccessor( sync *sync.Syncer[*header.ExtendedHeader], fraudServ libfraud.Service[*header.ExtendedHeader], opts []state.Option, -) (*state.CoreAccessor, Module, *modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]) { - ca := state.NewCoreAccessor(keyring, keyname, sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, opts...) +) ( + *state.CoreAccessor, + Module, + *modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader], + error, +) { + ca, err := state.NewCoreAccessor(keyring, keyname, sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, opts...) + if err != nil { + return nil, nil, nil, err + } + sBreaker := &modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]{ Service: ca, FraudType: byzantine.BadEncoding, FraudServ: fraudServ, } - return ca, ca, sBreaker + return ca, ca, sBreaker, nil } diff --git a/state/core_access.go b/state/core_access.go index 5108c2225e..15c489df88 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -39,16 +39,16 @@ import ( "github.com/celestiaorg/celestia-node/header" ) -var ( - log = logging.Logger("state") - ErrInvalidAmount = errors.New("state: amount must be greater than zero") -) - const ( - maxRetries = 5 - // gasMultiplier is used to increase gas limit in case if tx has additional options. gasMultiplier = 1.1 + maxRetries = 5 +) + +var ( + ErrInvalidAmount = errors.New("state: amount must be greater than zero") + + log = logging.Logger("state") ) // Option is the functional option that is applied to the coreAccessor instance @@ -257,11 +257,10 @@ func (ca *CoreAccessor) SubmitPayForBlob( minGasPrice := ca.getMinGasPrice() - var feeGrant apptypes.TxBuilderOption - + var feeGrant user.TxOption // set granter and update gasLimit in case node run in a grantee mode if !ca.granter.Empty() { - feeGrant = apptypes.SetFeeGranter(ca.granter) + feeGrant = user.SetFeeGranter(ca.granter) gasLim = uint64(float64(gasLim) * gasMultiplier) } // set the fee for the user as the minimum gas price multiplied by the gas limit @@ -273,7 +272,7 @@ func (ca *CoreAccessor) SubmitPayForBlob( var lastErr error for attempt := 0; attempt < maxRetries; attempt++ { - options := []apptypes.TxBuilderOption{apptypes.SetGasLimit(gasLim), withFee(fee)} + options := []user.TxOption{user.SetGasLimit(gasLim), withFee(fee)} if feeGrant != nil { options = append(options, feeGrant) } @@ -589,7 +588,12 @@ func (ca *CoreAccessor) GrantFee( fee Int, gasLim uint64, ) (*TxResponse, error) { - granter, err := ca.signer.GetSignerInfo().GetAddress() + signer, err := ca.getSigner(ctx) + if err != nil { + return nil, err + } + + granter, err := signer.GetSignerInfo().GetAddress() if err != nil { return nil, err } @@ -605,11 +609,7 @@ func (ca *CoreAccessor) GrantFee( return nil, nil } - signedTx, err := ca.constructSignedTx(ctx, msg, apptypes.SetGasLimit(gasLim), withFee(fee)) - if err != nil { - return nil, err - } - return ca.SubmitTx(ctx, signedTx) + return signer.SubmitTx(ctx, []sdktypes.Msg{msg}, user.SetGasLimit(gasLim), withFee(fee)) } func (ca *CoreAccessor) RevokeGrantFee( @@ -618,18 +618,18 @@ func (ca *CoreAccessor) RevokeGrantFee( fee Int, gasLim uint64, ) (*TxResponse, error) { - granter, err := ca.signer.GetSignerInfo().GetAddress() + signer, err := ca.getSigner(ctx) if err != nil { return nil, err } - msg := feegrant.NewMsgRevokeAllowance(granter, grantee) - signedTx, err := ca.constructSignedTx(ctx, &msg, apptypes.SetGasLimit(gasLim), withFee(fee)) + granter, err := signer.GetSignerInfo().GetAddress() if err != nil { return nil, err } - return ca.SubmitTx(ca.ctx, signedTx) + msg := feegrant.NewMsgRevokeAllowance(granter, grantee) + return signer.SubmitTx(ctx, []sdktypes.Msg{&msg}, user.SetGasLimit(gasLim), withFee(fee)) } func (ca *CoreAccessor) LastPayForBlob() int64 { @@ -696,3 +696,8 @@ func (ca *CoreAccessor) setupSigner(ctx context.Context) error { ca.signer, err = user.SetupSigner(ctx, ca.keyring, ca.coreConn, ca.addr, encCfg) return err } + +func withFee(fee Int) user.TxOption { + gasFee := sdktypes.NewCoins(sdktypes.NewCoin(app.BondDenom, fee)) + return user.SetFeeAmount(gasFee) +} From 2731cdc39d2f4336a9819b390201a2b7fb0f7a9e Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:49:23 +0200 Subject: [PATCH 08/13] fix(state): apply rebase corrections --- state/core_access.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/state/core_access.go b/state/core_access.go index 15c489df88..f8bc88139e 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -593,10 +593,7 @@ func (ca *CoreAccessor) GrantFee( return nil, err } - granter, err := signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } + granter := signer.Address() allowance := &feegrant.BasicAllowance{} if !amount.IsZero() { @@ -623,10 +620,7 @@ func (ca *CoreAccessor) RevokeGrantFee( return nil, err } - granter, err := signer.GetSignerInfo().GetAddress() - if err != nil { - return nil, err - } + granter := signer.Address() msg := feegrant.NewMsgRevokeAllowance(granter, grantee) return signer.SubmitTx(ctx, []sdktypes.Msg{&msg}, user.SetGasLimit(gasLim), withFee(fee)) From d6983a6bdabb3d90272faaf38433b1646f5a8af5 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Tue, 16 Apr 2024 16:02:30 +0200 Subject: [PATCH 09/13] fix(nodebuilder/state): provide as AccName not string --- nodebuilder/state/core.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nodebuilder/state/core.go b/nodebuilder/state/core.go index c31c338485..fd070b3dfd 100644 --- a/nodebuilder/state/core.go +++ b/nodebuilder/state/core.go @@ -18,7 +18,7 @@ import ( func coreAccessor( corecfg core.Config, keyring keyring.Keyring, - keyname string, + keyname AccName, sync *sync.Syncer[*header.ExtendedHeader], fraudServ libfraud.Service[*header.ExtendedHeader], opts []state.Option, @@ -28,7 +28,8 @@ func coreAccessor( *modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader], error, ) { - ca, err := state.NewCoreAccessor(keyring, keyname, sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, opts...) + ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, + opts...) if err != nil { return nil, nil, nil, err } From 24b8b2ff28b4eed8eac9fb3539cc116562a30803 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:02:56 +0200 Subject: [PATCH 10/13] fix(state): protect signer from potential race --- state/core_access.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/state/core_access.go b/state/core_access.go index f8bc88139e..8a59276a72 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -69,11 +69,12 @@ type CoreAccessor struct { cancel context.CancelFunc keyring keyring.Keyring + addr sdktypes.AccAddress // TODO: (@cmwaters) - once multiple keys within a signer is supported, // this will no longer be necessary. // ref: https://github.com/celestiaorg/celestia-app/issues/3259 - signer *user.Signer - addr sdktypes.AccAddress + signerMu sync.Mutex + signer *user.Signer getter libhead.Head[*header.ExtendedHeader] @@ -175,7 +176,7 @@ func (ca *CoreAccessor) Start(ctx context.Context) error { } ca.rpcCli = cli - err = ca.setupSigner(ctx) + ca.signer, err = ca.setupSigner(ctx) if err != nil { log.Warnw("failed to set up signer, check if node's account is funded", "err", err) } @@ -674,21 +675,21 @@ func (ca *CoreAccessor) queryMinimumGasPrice( } func (ca *CoreAccessor) getSigner(ctx context.Context) (*user.Signer, error) { - if ca.signer == nil { - err := ca.setupSigner(ctx) - if err != nil { - return nil, err - } + ca.signerMu.Lock() + defer ca.signerMu.Unlock() + + if ca.signer != nil { + return ca.signer, nil } - return ca.signer, nil + + var err error + ca.signer, err = ca.setupSigner(ctx) + return ca.signer, err } -func (ca *CoreAccessor) setupSigner(ctx context.Context) error { +func (ca *CoreAccessor) setupSigner(ctx context.Context) (*user.Signer, error) { encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) - - var err error - ca.signer, err = user.SetupSigner(ctx, ca.keyring, ca.coreConn, ca.addr, encCfg) - return err + return user.SetupSigner(ctx, ca.keyring, ca.coreConn, ca.addr, encCfg) } func withFee(fee Int) user.TxOption { From dcdf21bcf12d8c1c785a0eaf47432907ef5cb18e Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Wed, 17 Apr 2024 16:38:37 +0200 Subject: [PATCH 11/13] doc(core): Add docs to explain why lazy construction of signer --- state/core_access.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/state/core_access.go b/state/core_access.go index 8a59276a72..589ee973a2 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -674,6 +674,9 @@ func (ca *CoreAccessor) queryMinimumGasPrice( return coins.AmountOf(app.BondDenom).MustFloat64(), nil } +// getSigner returns the signer if it has already been constructed, otherwise +// it will attempt to set it up. The signer can only be constructed if the account +// exists / is funded. func (ca *CoreAccessor) getSigner(ctx context.Context) (*user.Signer, error) { ca.signerMu.Lock() defer ca.signerMu.Unlock() From 9585872181f1fd852db44b7661d22dc4c67d9d78 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Wed, 17 Apr 2024 16:45:48 +0200 Subject: [PATCH 12/13] chore(nodebuilder/state): slava nit --- nodebuilder/state/core.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/nodebuilder/state/core.go b/nodebuilder/state/core.go index fd070b3dfd..4b5d42ca43 100644 --- a/nodebuilder/state/core.go +++ b/nodebuilder/state/core.go @@ -30,14 +30,12 @@ func coreAccessor( ) { ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, opts...) - if err != nil { - return nil, nil, nil, err - } sBreaker := &modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]{ Service: ca, FraudType: byzantine.BadEncoding, FraudServ: fraudServ, } - return ca, ca, sBreaker, nil + + return ca, ca, sBreaker, err } From 3e1a15bed203112c014d4f7f1fef42c89d8f7e07 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Wed, 17 Apr 2024 16:49:56 +0200 Subject: [PATCH 13/13] chore(nodebuilder/state): rename to AccountName --- nodebuilder/state/core.go | 2 +- nodebuilder/state/keyring.go | 6 +++--- nodebuilder/state/module.go | 2 +- nodebuilder/state/opts.go | 2 +- nodebuilder/tests/swamp/swamp.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nodebuilder/state/core.go b/nodebuilder/state/core.go index 4b5d42ca43..591529ecbd 100644 --- a/nodebuilder/state/core.go +++ b/nodebuilder/state/core.go @@ -18,7 +18,7 @@ import ( func coreAccessor( corecfg core.Config, keyring keyring.Keyring, - keyname AccName, + keyname AccountName, sync *sync.Syncer[*header.ExtendedHeader], fraudServ libfraud.Service[*header.ExtendedHeader], opts []state.Option, diff --git a/nodebuilder/state/keyring.go b/nodebuilder/state/keyring.go index 25723ef5b7..1b65d27efe 100644 --- a/nodebuilder/state/keyring.go +++ b/nodebuilder/state/keyring.go @@ -8,12 +8,12 @@ import ( const DefaultAccountName = "my_celes_key" -type AccName string +type AccountName string // Keyring constructs a new keyring. // NOTE: we construct keyring before constructing node for easier UX // as having keyring-backend set to `file` prompts user for password. -func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, AccName, error) { +func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, AccountName, error) { ring := ks.Keyring() var info *kr.Record // if custom keyringAccName provided, find key for that name @@ -34,5 +34,5 @@ func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, AccName, error) { info = keyInfo } - return ring, AccName(info.Name), nil + return ring, AccountName(info.Name), nil } diff --git a/nodebuilder/state/module.go b/nodebuilder/state/module.go index 116a24ceda..29a1ed157e 100644 --- a/nodebuilder/state/module.go +++ b/nodebuilder/state/module.go @@ -31,7 +31,7 @@ func ConstructModule(tp node.Type, cfg *Config, coreCfg *core.Config) fx.Option fx.Supply(*cfg), fx.Error(cfgErr), fx.Supply(opts), - fx.Provide(func(ks keystore.Keystore) (keyring.Keyring, AccName, error) { + fx.Provide(func(ks keystore.Keystore) (keyring.Keyring, AccountName, error) { return Keyring(*cfg, ks) }), fxutil.ProvideIf(coreCfg.IsEndpointConfigured(), fx.Annotate( diff --git a/nodebuilder/state/opts.go b/nodebuilder/state/opts.go index 54a3530be9..cc3ee537c1 100644 --- a/nodebuilder/state/opts.go +++ b/nodebuilder/state/opts.go @@ -14,6 +14,6 @@ func WithKeyring(keyring kr.Keyring) fx.Option { } // WithKeyName configures the signer to use the given key. -func WithKeyName(name AccName) fx.Option { +func WithKeyName(name AccountName) fx.Option { return fx.Replace(name) } diff --git a/nodebuilder/tests/swamp/swamp.go b/nodebuilder/tests/swamp/swamp.go index 42fc0e4062..93879c4425 100644 --- a/nodebuilder/tests/swamp/swamp.go +++ b/nodebuilder/tests/swamp/swamp.go @@ -259,7 +259,7 @@ func (s *Swamp) NewNodeWithStore( ) *nodebuilder.Node { options = append(options, state.WithKeyring(s.ClientContext.Keyring), - state.WithKeyName(state.AccName(s.Accounts[0])), + state.WithKeyName(state.AccountName(s.Accounts[0])), ) switch tp {