From debad37cdd8db3858651b2c0506f7ef24fe43293 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 23 Jan 2024 17:06:50 -0600 Subject: [PATCH] refactor(blob)!: replace SubmitOptions with a GasPrice parameter (#3094) This PR modifies the blob interface to replace SubmitOptions with GasLimit. The reasoning here is that setting a gas price is a lot more intuitive/easier than calculating the fee and gas limit, which take cosmossdk and celestia-app imports to calculate without much effort. Related: https://github.com/rollkit/go-da/issues/30 Based on a suggestion from @vgonkivs we alias float64 to be able to provide a default value, but now that its there I think we can just have float64 and the same constructor, as it doesn't change that much. Breaks the API, cc @jcstein, @tuxcanfly, and @Ferret-san . Closes #3053. --- blob/service.go | 32 +++++++++++++++++++----- nodebuilder/blob/blob.go | 8 +++--- nodebuilder/blob/cmd/blob.go | 29 +++++++-------------- nodebuilder/blob/mocks/api.go | 5 ++-- nodebuilder/das/mocks/api.go | 3 +-- nodebuilder/fraud/mocks/api.go | 3 +-- nodebuilder/header/mocks/api.go | 3 +-- nodebuilder/node/mocks/api.go | 3 +-- nodebuilder/share/mocks/api.go | 13 +++++----- nodebuilder/state/mocks/api.go | 5 ++-- nodebuilder/tests/api_test.go | 2 +- nodebuilder/tests/blob_test.go | 6 ++--- share/availability/mocks/availability.go | 3 +-- share/mocks/getter.go | 3 +-- 14 files changed, 59 insertions(+), 59 deletions(-) diff --git a/blob/service.go b/blob/service.go index 79e7dd7937..6a70772797 100644 --- a/blob/service.go +++ b/blob/service.go @@ -4,13 +4,17 @@ import ( "context" "errors" "fmt" + "math" "sync" - "cosmossdk.io/math" + sdkmath "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/types" + auth "github.com/cosmos/cosmos-sdk/x/auth/types" logging "github.com/ipfs/go-log/v2" + "github.com/celestiaorg/celestia-app/pkg/appconsts" "github.com/celestiaorg/celestia-app/pkg/shares" + blobtypes "github.com/celestiaorg/celestia-app/x/blob/types" "github.com/celestiaorg/celestia-node/header" "github.com/celestiaorg/celestia-node/share" @@ -23,11 +27,21 @@ var ( log = logging.Logger("blob") ) +// GasPrice represents the amount to be paid per gas unit. Fee is set by +// multiplying GasPrice by GasLimit, which is determined by the blob sizes. +type GasPrice float64 + +// DefaultGasPrice returns the default gas price, letting node automatically +// determine the Fee based on the passed blob sizes. +func DefaultGasPrice() GasPrice { + return -1.0 +} + // Submitter is an interface that allows submitting blobs to the celestia-core. It is used to // avoid a circular dependency between the blob and the state package, since the state package needs // the blob.Blob type for this signature. type Submitter interface { - SubmitPayForBlob(ctx context.Context, fee math.Int, gasLim uint64, blobs []*Blob) (*types.TxResponse, error) + SubmitPayForBlob(ctx context.Context, fee sdkmath.Int, gasLim uint64, blobs []*Blob) (*types.TxResponse, error) } type Service struct { @@ -66,15 +80,21 @@ func DefaultSubmitOptions() *SubmitOptions { } } -// Submit sends PFB transaction and reports the height in which it was included. +// Submit sends PFB transaction and reports the height at which it was included. // Allows sending multiple Blobs atomically synchronously. // Uses default wallet registered on the Node. // Handles gas estimation and fee calculation. -func (s *Service) Submit(ctx context.Context, blobs []*Blob, options *SubmitOptions) (uint64, error) { +func (s *Service) Submit(ctx context.Context, blobs []*Blob, gasPrice GasPrice) (uint64, error) { log.Debugw("submitting blobs", "amount", len(blobs)) - if options == nil { - options = DefaultSubmitOptions() + options := DefaultSubmitOptions() + if gasPrice >= 0 { + blobSizes := make([]uint32, len(blobs)) + for i, blob := range blobs { + blobSizes[i] = uint32(len(blob.Data)) + } + options.GasLimit = blobtypes.EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, auth.DefaultTxSizeCostPerByte) + options.Fee = types.NewInt(int64(math.Ceil(float64(gasPrice) * float64(options.GasLimit)))).Int64() } resp, err := s.blobSubmitter.SubmitPayForBlob(ctx, types.NewInt(options.Fee), options.GasLimit, blobs) diff --git a/nodebuilder/blob/blob.go b/nodebuilder/blob/blob.go index 47ab255ac6..f87105541a 100644 --- a/nodebuilder/blob/blob.go +++ b/nodebuilder/blob/blob.go @@ -16,7 +16,7 @@ type Module interface { // Submit sends Blobs and reports the height in which they were included. // Allows sending multiple Blobs atomically synchronously. // Uses default wallet registered on the Node. - Submit(_ context.Context, _ []*blob.Blob, _ *blob.SubmitOptions) (height uint64, _ error) + Submit(_ context.Context, _ []*blob.Blob, _ blob.GasPrice) (height uint64, _ error) // Get retrieves the blob by commitment under the given namespace and height. Get(_ context.Context, height uint64, _ share.Namespace, _ blob.Commitment) (*blob.Blob, error) // GetAll returns all blobs at the given height under the given namespaces. @@ -30,7 +30,7 @@ type Module interface { type API struct { Internal struct { - Submit func(context.Context, []*blob.Blob, *blob.SubmitOptions) (uint64, error) `perm:"write"` + Submit func(context.Context, []*blob.Blob, blob.GasPrice) (uint64, error) `perm:"write"` Get func(context.Context, uint64, share.Namespace, blob.Commitment) (*blob.Blob, error) `perm:"read"` GetAll func(context.Context, uint64, []share.Namespace) ([]*blob.Blob, error) `perm:"read"` GetProof func(context.Context, uint64, share.Namespace, blob.Commitment) (*blob.Proof, error) `perm:"read"` @@ -38,8 +38,8 @@ type API struct { } } -func (api *API) Submit(ctx context.Context, blobs []*blob.Blob, options *blob.SubmitOptions) (uint64, error) { - return api.Internal.Submit(ctx, blobs, options) +func (api *API) Submit(ctx context.Context, blobs []*blob.Blob, gasPrice blob.GasPrice) (uint64, error) { + return api.Internal.Submit(ctx, blobs, gasPrice) } func (api *API) Get( diff --git a/nodebuilder/blob/cmd/blob.go b/nodebuilder/blob/cmd/blob.go index f3510edfe1..7862eaa204 100644 --- a/nodebuilder/blob/cmd/blob.go +++ b/nodebuilder/blob/cmd/blob.go @@ -16,8 +16,7 @@ import ( var ( base64Flag bool - fee int64 - gasLimit uint64 + gasPrice float64 ) func init() { @@ -37,23 +36,13 @@ func init() { "printed blob's data as a base64 string", ) - submitCmd.PersistentFlags().Int64Var( - &fee, - "fee", - -1, - "specifies fee (in utia) for blob submission.\n"+ - "Fee will be automatically calculated if negative value is passed [optional]", + submitCmd.PersistentFlags().Float64Var( + &gasPrice, + "gas.price", + float64(blob.DefaultGasPrice()), + "specifies gas price (in utia) for blob submission.\n"+ + "Gas price will be set to default (0.002) if no value is passed", ) - - submitCmd.PersistentFlags().Uint64Var( - &gasLimit, - "gas.limit", - 0, - "sets the amount of gas that is consumed during blob submission [optional]", - ) - - // unset the default value to avoid users confusion - submitCmd.PersistentFlags().Lookup("fee").DefValue = "0" } var Cmd = &cobra.Command{ @@ -135,7 +124,7 @@ var submitCmd = &cobra.Command{ Short: "Submit the blob at the given namespace.\n" + "Note:\n" + "* only one blob is allowed to submit through the RPC.\n" + - "* fee and gas.limit params will be calculated automatically if they are not provided as arguments", + "* fee and gas limit params will be calculated automatically.\n", RunE: func(cmd *cobra.Command, args []string) error { client, err := cmdnode.ParseClientFromCtx(cmd.Context()) if err != nil { @@ -156,7 +145,7 @@ var submitCmd = &cobra.Command{ height, err := client.Blob.Submit( cmd.Context(), []*blob.Blob{parsedBlob}, - &blob.SubmitOptions{Fee: fee, GasLimit: gasLimit}, + blob.GasPrice(gasPrice), ) response := struct { diff --git a/nodebuilder/blob/mocks/api.go b/nodebuilder/blob/mocks/api.go index 40bf299da1..0898e70459 100644 --- a/nodebuilder/blob/mocks/api.go +++ b/nodebuilder/blob/mocks/api.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - blob "github.com/celestiaorg/celestia-node/blob" share "github.com/celestiaorg/celestia-node/share" + gomock "github.com/golang/mock/gomock" ) // MockModule is a mock of Module interface. @@ -98,7 +97,7 @@ func (mr *MockModuleMockRecorder) Included(arg0, arg1, arg2, arg3, arg4 interfac } // Submit mocks base method. -func (m *MockModule) Submit(arg0 context.Context, arg1 []*blob.Blob, arg2 *blob.SubmitOptions) (uint64, error) { +func (m *MockModule) Submit(arg0 context.Context, arg1 []*blob.Blob, arg2 blob.GasPrice) (uint64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Submit", arg0, arg1, arg2) ret0, _ := ret[0].(uint64) diff --git a/nodebuilder/das/mocks/api.go b/nodebuilder/das/mocks/api.go index 68ffaf3c8c..c4046e90e8 100644 --- a/nodebuilder/das/mocks/api.go +++ b/nodebuilder/das/mocks/api.go @@ -8,9 +8,8 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - das "github.com/celestiaorg/celestia-node/das" + gomock "github.com/golang/mock/gomock" ) // MockModule is a mock of Module interface. diff --git a/nodebuilder/fraud/mocks/api.go b/nodebuilder/fraud/mocks/api.go index 10111b81a8..fcc7a58231 100644 --- a/nodebuilder/fraud/mocks/api.go +++ b/nodebuilder/fraud/mocks/api.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - fraud "github.com/celestiaorg/celestia-node/nodebuilder/fraud" fraud0 "github.com/celestiaorg/go-fraud" + gomock "github.com/golang/mock/gomock" ) // MockModule is a mock of Module interface. diff --git a/nodebuilder/header/mocks/api.go b/nodebuilder/header/mocks/api.go index 9b15f2242e..b0d2b961d9 100644 --- a/nodebuilder/header/mocks/api.go +++ b/nodebuilder/header/mocks/api.go @@ -8,11 +8,10 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - header "github.com/celestiaorg/celestia-node/header" header0 "github.com/celestiaorg/go-header" sync "github.com/celestiaorg/go-header/sync" + gomock "github.com/golang/mock/gomock" ) // MockModule is a mock of Module interface. diff --git a/nodebuilder/node/mocks/api.go b/nodebuilder/node/mocks/api.go index 14357316dc..d8789a771c 100644 --- a/nodebuilder/node/mocks/api.go +++ b/nodebuilder/node/mocks/api.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" + node "github.com/celestiaorg/celestia-node/nodebuilder/node" auth "github.com/filecoin-project/go-jsonrpc/auth" gomock "github.com/golang/mock/gomock" - - node "github.com/celestiaorg/celestia-node/nodebuilder/node" ) // MockModule is a mock of Module interface. diff --git a/nodebuilder/share/mocks/api.go b/nodebuilder/share/mocks/api.go index 87f98e4f73..4e21cecae0 100644 --- a/nodebuilder/share/mocks/api.go +++ b/nodebuilder/share/mocks/api.go @@ -8,11 +8,10 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - - da "github.com/celestiaorg/celestia-app/pkg/da" + header "github.com/celestiaorg/celestia-node/header" share "github.com/celestiaorg/celestia-node/share" rsmt2d "github.com/celestiaorg/rsmt2d" + gomock "github.com/golang/mock/gomock" ) // MockModule is a mock of Module interface. @@ -39,7 +38,7 @@ func (m *MockModule) EXPECT() *MockModuleMockRecorder { } // GetEDS mocks base method. -func (m *MockModule) GetEDS(arg0 context.Context, arg1 *da.DataAvailabilityHeader) (*rsmt2d.ExtendedDataSquare, error) { +func (m *MockModule) GetEDS(arg0 context.Context, arg1 *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetEDS", arg0, arg1) ret0, _ := ret[0].(*rsmt2d.ExtendedDataSquare) @@ -54,7 +53,7 @@ func (mr *MockModuleMockRecorder) GetEDS(arg0, arg1 interface{}) *gomock.Call { } // GetShare mocks base method. -func (m *MockModule) GetShare(arg0 context.Context, arg1 *da.DataAvailabilityHeader, arg2, arg3 int) ([]byte, error) { +func (m *MockModule) GetShare(arg0 context.Context, arg1 *header.ExtendedHeader, arg2, arg3 int) ([]byte, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetShare", arg0, arg1, arg2, arg3) ret0, _ := ret[0].([]byte) @@ -69,7 +68,7 @@ func (mr *MockModuleMockRecorder) GetShare(arg0, arg1, arg2, arg3 interface{}) * } // GetSharesByNamespace mocks base method. -func (m *MockModule) GetSharesByNamespace(arg0 context.Context, arg1 *da.DataAvailabilityHeader, arg2 share.Namespace) (share.NamespacedShares, error) { +func (m *MockModule) GetSharesByNamespace(arg0 context.Context, arg1 *header.ExtendedHeader, arg2 share.Namespace) (share.NamespacedShares, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetSharesByNamespace", arg0, arg1, arg2) ret0, _ := ret[0].(share.NamespacedShares) @@ -84,7 +83,7 @@ func (mr *MockModuleMockRecorder) GetSharesByNamespace(arg0, arg1, arg2 interfac } // SharesAvailable mocks base method. -func (m *MockModule) SharesAvailable(arg0 context.Context, arg1 *da.DataAvailabilityHeader) error { +func (m *MockModule) SharesAvailable(arg0 context.Context, arg1 *header.ExtendedHeader) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SharesAvailable", arg0, arg1) ret0, _ := ret[0].(error) diff --git a/nodebuilder/state/mocks/api.go b/nodebuilder/state/mocks/api.go index 6499a6dfd8..1861a86e66 100644 --- a/nodebuilder/state/mocks/api.go +++ b/nodebuilder/state/mocks/api.go @@ -9,13 +9,12 @@ import ( reflect "reflect" math "cosmossdk.io/math" + blob "github.com/celestiaorg/celestia-node/blob" + state "github.com/celestiaorg/celestia-node/state" types "github.com/cosmos/cosmos-sdk/types" types0 "github.com/cosmos/cosmos-sdk/x/staking/types" gomock "github.com/golang/mock/gomock" types1 "github.com/tendermint/tendermint/types" - - blob "github.com/celestiaorg/celestia-node/blob" - state "github.com/celestiaorg/celestia-node/state" ) // MockModule is a mock of Module interface. diff --git a/nodebuilder/tests/api_test.go b/nodebuilder/tests/api_test.go index a793ba7b33..960ce38875 100644 --- a/nodebuilder/tests/api_test.go +++ b/nodebuilder/tests/api_test.go @@ -113,7 +113,7 @@ func TestBlobRPC(t *testing.T) { ) require.NoError(t, err) - height, err := rpcClient.Blob.Submit(ctx, []*blob.Blob{newBlob}, nil) + height, err := rpcClient.Blob.Submit(ctx, []*blob.Blob{newBlob}, blob.DefaultGasPrice()) require.NoError(t, err) require.True(t, height != 0) } diff --git a/nodebuilder/tests/blob_test.go b/nodebuilder/tests/blob_test.go index a30e91b2e6..d0aeefd568 100644 --- a/nodebuilder/tests/blob_test.go +++ b/nodebuilder/tests/blob_test.go @@ -60,7 +60,7 @@ func TestBlobModule(t *testing.T) { fullClient := getAdminClient(ctx, fullNode, t) lightClient := getAdminClient(ctx, lightNode, t) - height, err := fullClient.Blob.Submit(ctx, blobs, nil) + height, err := fullClient.Blob.Submit(ctx, blobs, blob.DefaultGasPrice()) require.NoError(t, err) _, err = fullClient.Header.WaitForHeight(ctx, height) @@ -143,7 +143,7 @@ func TestBlobModule(t *testing.T) { ) require.NoError(t, err) - height, err := fullClient.Blob.Submit(ctx, []*blob.Blob{b, b}, nil) + height, err := fullClient.Blob.Submit(ctx, []*blob.Blob{b, b}, blob.DefaultGasPrice()) require.NoError(t, err) _, err = fullClient.Header.WaitForHeight(ctx, height) @@ -172,7 +172,7 @@ func TestBlobModule(t *testing.T) { // different pfbs. name: "Submit the same blob in different pfb", doFn: func(t *testing.T) { - h, err := fullClient.Blob.Submit(ctx, []*blob.Blob{blobs[0]}, nil) + h, err := fullClient.Blob.Submit(ctx, []*blob.Blob{blobs[0]}, blob.DefaultGasPrice()) require.NoError(t, err) _, err = fullClient.Header.WaitForHeight(ctx, h) diff --git a/share/availability/mocks/availability.go b/share/availability/mocks/availability.go index bb1c01bf85..fc68d3d2bc 100644 --- a/share/availability/mocks/availability.go +++ b/share/availability/mocks/availability.go @@ -8,9 +8,8 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - header "github.com/celestiaorg/celestia-node/header" + gomock "github.com/golang/mock/gomock" ) // MockAvailability is a mock of Availability interface. diff --git a/share/mocks/getter.go b/share/mocks/getter.go index 2adfa50cfe..738e2b246c 100644 --- a/share/mocks/getter.go +++ b/share/mocks/getter.go @@ -8,11 +8,10 @@ import ( context "context" reflect "reflect" - gomock "github.com/golang/mock/gomock" - header "github.com/celestiaorg/celestia-node/header" share "github.com/celestiaorg/celestia-node/share" rsmt2d "github.com/celestiaorg/rsmt2d" + gomock "github.com/golang/mock/gomock" ) // MockGetter is a mock of Getter interface.