Skip to content

feat(state/core_accessor): add fee estimator #4087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

feat(state/core_accessor): add fee estimator #4087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
744b7f9
feat: add fee estimator
vgonkivs Feb 4, 2025
0e63366
fix test
vgonkivs Feb 13, 2025
a5ecdaa
fix go mod
vgonkivs Feb 14, 2025
539f554
refactoring
vgonkivs Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/docgen/examples.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ func init() {
state.WithKeyName("my_celes_key"),
state.WithSignerAddress("celestia1pjcmwj8w6hyr2c4wehakc5g8cfs36aysgucx66"),
state.WithFeeGranterAddress("celestia1hakc56ax66ypjcmwj8w6hyr2c4g8cfs3wesguc"),
state.WithMaxGasPrice(state.DefaultMaxGasPrice),
state.WithTxPriority(1),
))

add(network.DirUnknown)
Expand Down
14 changes: 10 additions & 4 deletions libs/utils/address.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,20 @@ import (

var ErrInvalidIP = errors.New("invalid IP address or hostname given")

// SanitizeAddr trims leading protocol scheme and port from the given
// IP address or hostname if present.
func SanitizeAddr(addr string) (string, error) {
original := addr
// NormalizeAddress extracts the host and port, removing unsupported schemes.
func NormalizeAddress(addr string) string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does SanitizeAddr not work for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it additionally cuts the port.
addr = strings.Split(addr, ":")[0]

addr = strings.TrimPrefix(addr, "http://")
addr = strings.TrimPrefix(addr, "https://")
addr = strings.TrimPrefix(addr, "tcp://")
addr = strings.TrimSuffix(addr, "/")
return addr
}

// SanitizeAddr trims leading protocol scheme and port from the given
// IP address or hostname if present.
func SanitizeAddr(addr string) (string, error) {
original := addr
addr = NormalizeAddress(addr)
addr = strings.Split(addr, ":")[0]
if addr == "" {
return "", fmt.Errorf("%w: %s", ErrInvalidIP, original)
Expand Down
6 changes: 6 additions & 0 deletions nodebuilder/core/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const (

var MetricsEnabled bool

type EstimatorAddress string

// Config combines all configuration fields for managing the relationship with a Core node.
type Config struct {
IP string
Expand All @@ -24,6 +26,8 @@ type Config struct {
// The JSON file should have a key-value pair where the key is "x-token" and the value is the authentication token.
// If left empty, the client will not include the X-Token in its requests.
XTokenPath string
// FeeEstimatorAddress specifies a third-party endpoint that will be used to calculate the gas price and gas.
FeeEstimatorAddress EstimatorAddress
}

// DefaultConfig returns default configuration for managing the
Expand Down Expand Up @@ -54,6 +58,8 @@ func (cfg *Config) Validate() error {
if err != nil {
return fmt.Errorf("nodebuilder/core: invalid grpc port: %s", err.Error())
}
pasedAddr := utils.NormalizeAddress(string(cfg.FeeEstimatorAddress))
cfg.FeeEstimatorAddress = EstimatorAddress(pasedAddr)
return nil
}

Expand Down
23 changes: 18 additions & 5 deletions nodebuilder/core/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import (
)

var (
coreIPFlag = "core.ip"
corePortFlag = "core.port"
coreGRPCFlag = "core.grpc.port"
coreTLS = "core.tls"
coreXTokenPathFlag = "core.xtoken.path" //nolint:gosec
coreIPFlag = "core.ip"
corePortFlag = "core.port"
coreGRPCFlag = "core.grpc.port"
coreTLS = "core.tls"
coreXTokenPathFlag = "core.xtoken.path" //nolint:gosec
coreEstimatorAddressFlag = "core.estimator.address"
)

// Flags gives a set of hardcoded Core flags.
Expand Down Expand Up @@ -50,6 +51,13 @@ func Flags() *flag.FlagSet {
"NOTE: the path is parsed only if coreTLS enabled."+
"If left empty, the client will not include the X-Token in its requests.",
)
flags.String(
coreEstimatorAddressFlag,
"",
"specifies the endpoint of the third-party service that should be used to calculate"+
"the gas price and gas. Format: <address>:<port>. Default connection to the consensus node will be used if "+
"left empty.",
)
return flags
}

Expand Down Expand Up @@ -88,5 +96,10 @@ func ParseFlags(
}
}
cfg.IP = coreIP

if cmd.Flag(coreEstimatorAddressFlag).Changed {
addr := cmd.Flag(coreEstimatorAddressFlag).Value.String()
cfg.FeeEstimatorAddress = EstimatorAddress(addr)
}
return cfg.Validate()
}
24 changes: 24 additions & 0 deletions nodebuilder/state/cmd/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"strconv"
"strings"

"cosmossdk.io/math"
"github.com/spf13/cobra"
Expand All @@ -18,6 +19,8 @@ var (
gasPrice float64
feeGranterAddress string
amount uint64
txPriority int
maxGasPrice float64
)

func init() {
Expand Down Expand Up @@ -460,6 +463,25 @@ func ApplyFlags(cmds ...*cobra.Command) {
"Note: The granter should be provided as a Bech32 address.\n"+
"Example: celestiaxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
)

// add additional flags for all submit transactions besides submit blobs.
if !strings.Contains(cmd.Name(), "blob") {
cmd.PersistentFlags().Float64Var(
&maxGasPrice,
"max.gas.price",
state.DefaultMaxGasPrice,
"Specifies max gas price for the tx submission.",
)
cmd.PersistentFlags().IntVar(
&txPriority,
"tx.priority",
state.TxPriorityMedium,
"Specifies tx priority. Should be set in range:"+
"1. TxPriorityLow;\n"+
"2. TxPriorityMedium;\n"+
"3. TxPriorityHigh.\nDefault: TxPriorityMedium",
)
}
}
}

Expand All @@ -470,5 +492,7 @@ func GetTxConfig() *state.TxConfig {
state.WithKeyName(keyName),
state.WithSignerAddress(signer),
state.WithFeeGranterAddress(feeGranterAddress),
state.WithMaxGasPrice(maxGasPrice),
state.WithTxPriority(txPriority),
)
}
4 changes: 3 additions & 1 deletion nodebuilder/state/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/celestiaorg/go-header/sync"

"github.com/celestiaorg/celestia-node/header"
"github.com/celestiaorg/celestia-node/nodebuilder/core"
modfraud "github.com/celestiaorg/celestia-node/nodebuilder/fraud"
"github.com/celestiaorg/celestia-node/nodebuilder/p2p"
"github.com/celestiaorg/celestia-node/share/eds/byzantine"
Expand All @@ -23,13 +24,14 @@ func coreAccessor(
fraudServ libfraud.Service[*header.ExtendedHeader],
network p2p.Network,
client *grpc.ClientConn,
address core.EstimatorAddress,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you just grab the core cfg there and take from cfg directly instead of supplying individual value to fx?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using core.EstimatorAddress only from the core config:

type Config struct {
	IP   string
	Port string
	TLSEnabled bool
	XTokenPath string
	FeeEstimatorAddress EstimatorAddress
}

) (
*state.CoreAccessor,
Module,
*modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader],
error,
) {
ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, client, network.String())
ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, client, network.String(), string(address))

sBreaker := &modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]{
Service: ca,
Expand Down
1 change: 1 addition & 0 deletions nodebuilder/state/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func ConstructModule(tp node.Type, cfg *Config, coreCfg *core.Config) fx.Option
cfgErr := cfg.Validate()
baseComponents := fx.Options(
fx.Supply(*cfg),
fx.Supply(coreCfg.FeeEstimatorAddress),
fx.Error(cfgErr),
fx.Provide(func(ks keystore.Keystore) (keyring.Keyring, AccountName, error) {
return Keyring(*cfg, ks)
Expand Down
46 changes: 25 additions & 21 deletions state/core_access.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ const (

var (
ErrInvalidAmount = errors.New("state: amount must be greater than zero")

log = logging.Logger("state")
log = logging.Logger("state")
)

// CoreAccessor implements service over a gRPC connection
Expand All @@ -63,6 +62,7 @@ type CoreAccessor struct {
coreConn *grpc.ClientConn
network string

estimator *estimator
// these fields are mutatable and thus need to be protected by a mutex
lock sync.Mutex
lastPayForBlob int64
Expand All @@ -83,6 +83,7 @@ func NewCoreAccessor(
getter libhead.Head[*header.ExtendedHeader],
conn *grpc.ClientConn,
network string,
estimatorAddress string,
) (*CoreAccessor, error) {
// create verifier
prt := merkle.DefaultProofRuntime()
Expand All @@ -106,6 +107,7 @@ func NewCoreAccessor(
prt: prt,
coreConn: conn,
network: network,
estimator: &estimator{estimatorAddress: estimatorAddress, defaultClientConn: conn},
}
return ca, nil
}
Expand Down Expand Up @@ -136,12 +138,17 @@ func (ca *CoreAccessor) Start(ctx context.Context) error {
if err != nil {
return fmt.Errorf("querying minimum gas price: %w", err)
}

err = ca.estimator.Start(ctx)
if err != nil {
log.Warn("state: failed to connect to estimator endpoint", "err", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we return err here? if no - comment why it's ok to skip it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should actually return an error here bc the only time err is returned from estimator.Start is if grpc conn fails to be created which is an actual err.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return nil
}

func (ca *CoreAccessor) Stop(context.Context) error {
func (ca *CoreAccessor) Stop(ctx context.Context) error {
ca.cancel()
return nil
return ca.estimator.Stop(ctx)
}

// SubmitPayForBlob builds, signs, and synchronously submits a MsgPayForBlob with additional
Expand Down Expand Up @@ -176,7 +183,7 @@ func (ca *CoreAccessor) SubmitPayForBlob(
for i, blob := range libBlobs {
blobSizes[i] = uint32(len(blob.Data()))
}
gas = estimateGasForBlobs(blobSizes)
gas = ca.estimator.estimateGasForBlobs(blobSizes)
}

gasPrice := cfg.GasPrice()
Expand Down Expand Up @@ -574,22 +581,6 @@ func (ca *CoreAccessor) submitMsg(
}

txConfig := make([]user.TxOption, 0)
gas := cfg.GasLimit()

if gas == 0 {
gas, err = estimateGas(ctx, client, msg)
if err != nil {
return nil, fmt.Errorf("estimating gas: %w", err)
}
}

gasPrice := cfg.GasPrice()
if gasPrice == DefaultGasPrice {
gasPrice = ca.minGasPrice
}

txConfig = append(txConfig, user.SetGasLimitAndGasPrice(gas, gasPrice))

if cfg.FeeGranterAddress() != "" {
granter, err := parseAccAddressFromString(cfg.FeeGranterAddress())
if err != nil {
Expand All @@ -598,7 +589,20 @@ func (ca *CoreAccessor) submitMsg(
txConfig = append(txConfig, user.SetFeeGranter(granter))
}

gasPrice, gas, err := ca.estimator.estimate(ctx, cfg, client, msg)
if err != nil {
return nil, err
}

if gasPrice < ca.getMinGasPrice() {
gasPrice = ca.getMinGasPrice()
}
txConfig = append(txConfig, user.SetGasLimitAndGasPrice(gas, gasPrice))

resp, err := client.SubmitTx(ctx, []sdktypes.Msg{msg}, txConfig...)
if err != nil {
return nil, err
}
return convertToSdkTxResponse(resp), err
}

Expand Down
17 changes: 15 additions & 2 deletions state/core_access_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,19 @@ func TestTransfer(t *testing.T) {
expErr: nil,
},
{
name: "transfer with options",
name: "transfer with gasPrice set",
gasPrice: 0.005,
gasLim: 0,
account: accounts[2],
expErr: nil,
},
{
name: "transfer with gas set",
gasPrice: DefaultGasPrice,
gasLim: 84617,
account: accounts[2],
expErr: nil,
},
}

for _, tc := range testcases {
Expand Down Expand Up @@ -213,6 +220,12 @@ func TestDelegate(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, 0, resp.Code)

opts = NewTxConfig(
WithGas(tc.gasLim),
WithGasPrice(tc.gasPrice),
WithKeyName(accounts[2]),
)

resp, err = ca.Undelegate(ctx, ValAddress(valAddr), sdktypes.NewInt(100_000), opts)
require.NoError(t, err)
require.EqualValues(t, 0, resp.Code)
Expand Down Expand Up @@ -265,7 +278,7 @@ func buildAccessor(t *testing.T) (*CoreAccessor, []string) {

conn, err := grpc.NewClient(grpcAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.NoError(t, err)
ca, err := NewCoreAccessor(cctx.Keyring, accounts[0].Name, nil, conn, chainID)
ca, err := NewCoreAccessor(cctx.Keyring, accounts[0].Name, nil, conn, chainID, "")
require.NoError(t, err)
return ca, getNames(accounts)
}
Expand Down
Loading
Loading