Skip to content

Commit

Permalink
Allow non-hash based blocknum argument in CeloAPI backend (#138)
Browse files Browse the repository at this point in the history 
* Allow non-hash based blocknum argument in CeloAPI backend

* Remove LRU cache from GetExchangeRates

Lets keep it simple and not do any premature optimization.
If we implement this it might be better to do it on a lower level
anyways.
  • Loading branch information
@ezdac @carterqw2
ezdac authored and carterqw2 committed Jun 11, 2024
1 parent 99f0c6a commit 74cfc8c
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 51 deletions.
42 changes: 13 additions & 29 deletions internal/celoapi/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,85 +7,69 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/exchange"
"github.com/ethereum/go-ethereum/common/lru"
"github.com/ethereum/go-ethereum/contracts"
"github.com/ethereum/go-ethereum/internal/ethapi"
"github.com/ethereum/go-ethereum/rpc"
)

func NewCeloAPIBackend(b ethapi.Backend) *CeloAPIBackend {
return &CeloAPIBackend{
Backend: b,
exchangeRatesCache: lru.NewCache[common.Hash, common.ExchangeRates](128),
Backend: b,
}
}

// CeloAPIBackend is a wrapper for the ethapi.Backend, that provides additional Celo specific
// functionality. CeloAPIBackend is mainly passed to the JSON RPC services and provides
// an easy way to make extra functionality available in the service internal methods without
// having to change their call signature significantly.
// CeloAPIBackend keeps a threadsafe LRU cache of block-hash to exchange rates for that block.
// Cache invalidation is only a problem when an already existing blocks' hash
// doesn't change, but the rates change. That shouldn't be possible, since changing the rates
// requires different transaction hashes / state and thus a different block hash.
// If the previous rates change during a reorg, the previous block hash should also change
// and with it the new block's hash.
// Stale branches cache values will get evicted eventually.
type CeloAPIBackend struct {
ethapi.Backend

exchangeRatesCache *lru.Cache[common.Hash, common.ExchangeRates]
}

func (b *CeloAPIBackend) getContractCaller(ctx context.Context, atBlock common.Hash) (*contracts.CeloBackend, error) {
func (b *CeloAPIBackend) getContractCaller(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash) (*contracts.CeloBackend, error) {
state, _, err := b.Backend.StateAndHeaderByNumberOrHash(
ctx,
rpc.BlockNumberOrHashWithHash(atBlock, false),
blockNumOrHash,
)
if err != nil {
return nil, fmt.Errorf("retrieve state for block hash %s: %w", atBlock.String(), err)
return nil, fmt.Errorf("retrieve state for block hash %s: %w", blockNumOrHash.String(), err)
}
return &contracts.CeloBackend{
ChainConfig: b.Backend.ChainConfig(),
State: state,
}, nil
}

func (b *CeloAPIBackend) GetFeeBalance(ctx context.Context, atBlock common.Hash, account common.Address, feeCurrency *common.Address) (*big.Int, error) {
cb, err := b.getContractCaller(ctx, atBlock)
func (b *CeloAPIBackend) GetFeeBalance(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, account common.Address, feeCurrency *common.Address) (*big.Int, error) {
cb, err := b.getContractCaller(ctx, blockNumOrHash)
if err != nil {
return nil, err
}
return contracts.GetFeeBalance(cb, account, feeCurrency), nil
}

func (b *CeloAPIBackend) GetExchangeRates(ctx context.Context, atBlock common.Hash) (common.ExchangeRates, error) {
cachedRates, ok := b.exchangeRatesCache.Get(atBlock)
if ok {
return cachedRates, nil
}
cb, err := b.getContractCaller(ctx, atBlock)
func (b *CeloAPIBackend) GetExchangeRates(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash) (common.ExchangeRates, error) {
contractBackend, err := b.getContractCaller(ctx, blockNumOrHash)
if err != nil {
return nil, err
}
er, err := contracts.GetExchangeRates(cb)
er, err := contracts.GetExchangeRates(contractBackend)
if err != nil {
return nil, err
}
b.exchangeRatesCache.Add(atBlock, er)
return er, nil
}

func (b *CeloAPIBackend) ConvertToCurrency(ctx context.Context, atBlock common.Hash, value *big.Int, fromFeeCurrency *common.Address) (*big.Int, error) {
er, err := b.GetExchangeRates(ctx, atBlock)
func (b *CeloAPIBackend) ConvertToCurrency(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, fromFeeCurrency *common.Address) (*big.Int, error) {
er, err := b.GetExchangeRates(ctx, blockNumOrHash)
if err != nil {
return nil, err
}
return exchange.ConvertGoldToCurrency(er, fromFeeCurrency, value)
}

func (b *CeloAPIBackend) ConvertToGold(ctx context.Context, atBlock common.Hash, value *big.Int, toFeeCurrency *common.Address) (*big.Int, error) {
er, err := b.GetExchangeRates(ctx, atBlock)
func (b *CeloAPIBackend) ConvertToGold(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, toFeeCurrency *common.Address) (*big.Int, error) {
er, err := b.GetExchangeRates(ctx, blockNumOrHash)
if err != nil {
return nil, err
}
Expand Down
11 changes: 6 additions & 5 deletions internal/ethapi/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1286,12 +1286,13 @@ func DoEstimateGas(ctx context.Context, b CeloBackend, args TransactionArgs, blo
State: state,
ErrorRatio: estimateGasErrorRatio,
}

// Celo specific: get exchange rates if fee currency is specified
exchangeRates := emptyExchangeRates
if args.FeeCurrency != nil {
// It is debatable whether we should use Hash or ParentHash here. Usually,
// user would probably like the recent rates after the block, so we use Hash.
exchangeRates, err = b.GetExchangeRates(ctx, header.Hash())
// It is debatable whether we should use the block itself or the parent block here.
// Usually, user would probably like the recent rates after the block, so we use the block itself.
exchangeRates, err = b.GetExchangeRates(ctx, blockNrOrHash)
if err != nil {
return 0, fmt.Errorf("get exchange rates for block: %v err: %w", header.Hash(), err)
}
Expand All @@ -1302,7 +1303,7 @@ func DoEstimateGas(ctx context.Context, b CeloBackend, args TransactionArgs, blo
return 0, err
}
// Celo specific: get balance
balance, err := b.GetFeeBalance(ctx, opts.Header.Hash(), call.From, args.FeeCurrency)
balance, err := b.GetFeeBalance(ctx, blockNrOrHash, call.From, args.FeeCurrency)
if err != nil {
return 0, err
}
Expand Down Expand Up @@ -1725,7 +1726,7 @@ func AccessList(ctx context.Context, b CeloBackend, blockNrOrHash rpc.BlockNumbe
// Always use the header's parent here, since we want to create the list at the
// queried block, but want to use the exchange rates before (at the beginning of)
// the queried block
exchangeRates, err = b.GetExchangeRates(ctx, header.ParentHash)
exchangeRates, err = b.GetExchangeRates(ctx, rpc.BlockNumberOrHashWithHash(header.ParentHash, false))
if err != nil {
return nil, 0, nil, fmt.Errorf("get exchange rates for block: %v err: %w", header.Hash(), err)
}
Expand Down
10 changes: 5 additions & 5 deletions internal/ethapi/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,9 @@ type celoTestBackend struct {
*testBackend
}

func (c *celoTestBackend) GetFeeBalance(ctx context.Context, atBlock common.Hash, account common.Address, feeCurrency *common.Address) (*big.Int, error) {
func (c *celoTestBackend) GetFeeBalance(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, account common.Address, feeCurrency *common.Address) (*big.Int, error) {
if feeCurrency == nil {
header, err := c.HeaderByHash(ctx, atBlock)
header, err := c.HeaderByNumberOrHash(ctx, blockNumOrHash)
if err != nil {
return nil, fmt.Errorf("retrieve header by hash in testBackend: %w", err)
}
Expand All @@ -607,20 +607,20 @@ func (c *celoTestBackend) GetFeeBalance(ctx context.Context, atBlock common.Hash
return nil, errCeloNotImplemented
}

func (c *celoTestBackend) GetExchangeRates(ctx context.Context, atBlock common.Hash) (common.ExchangeRates, error) {
func (c *celoTestBackend) GetExchangeRates(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash) (common.ExchangeRates, error) {
var er common.ExchangeRates
return er, nil
}

func (c *celoTestBackend) ConvertToCurrency(ctx context.Context, atBlock common.Hash, value *big.Int, feeCurrency *common.Address) (*big.Int, error) {
func (c *celoTestBackend) ConvertToCurrency(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, feeCurrency *common.Address) (*big.Int, error) {
if feeCurrency == nil {
return value, nil
}
// Celo specific backend features are currently not tested
return nil, errCeloNotImplemented
}

func (c *celoTestBackend) ConvertToGold(ctx context.Context, atBlock common.Hash, value *big.Int, feeCurrency *common.Address) (*big.Int, error) {
func (c *celoTestBackend) ConvertToGold(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, feeCurrency *common.Address) (*big.Int, error) {
if feeCurrency == nil {
return value, nil
}
Expand Down
8 changes: 4 additions & 4 deletions internal/ethapi/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ import (
type CeloBackend interface {
Backend

GetFeeBalance(ctx context.Context, atBlock common.Hash, account common.Address, feeCurrency *common.Address) (*big.Int, error)
GetExchangeRates(ctx context.Context, atBlock common.Hash) (common.ExchangeRates, error)
ConvertToCurrency(ctx context.Context, atBlock common.Hash, value *big.Int, feeCurrency *common.Address) (*big.Int, error)
ConvertToGold(ctx context.Context, atBlock common.Hash, value *big.Int, feeCurrency *common.Address) (*big.Int, error)
GetFeeBalance(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, account common.Address, feeCurrency *common.Address) (*big.Int, error)
GetExchangeRates(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (common.ExchangeRates, error)
ConvertToCurrency(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, value *big.Int, feeCurrency *common.Address) (*big.Int, error)
ConvertToGold(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, value *big.Int, feeCurrency *common.Address) (*big.Int, error)
}

// Backend interface provides the common API services (that are provided by both full and light clients) with access to necessary functions.
Expand Down
28 changes: 24 additions & 4 deletions internal/ethapi/transaction_args.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,12 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b CeloBackend)
return err
}
if args.IsFeeCurrencyDenominated() {
price, err = b.ConvertToCurrency(ctx, head.Hash(), price, args.FeeCurrency)
price, err = b.ConvertToCurrency(
ctx,
rpc.BlockNumberOrHashWithHash(head.Hash(), false),
price,
args.FeeCurrency,
)
if err != nil {
return fmt.Errorf("can't convert suggested gasTipCap to fee-currency: %w", err)
}
Expand All @@ -272,7 +277,12 @@ func (args *TransactionArgs) setCancunFeeDefaults(ctx context.Context, head *typ
// wether the blob-fee will be used like that in Cel2 or not,
// at least this keeps it consistent with the rest of the gas-fees
var err error
blobBaseFee, err = b.ConvertToCurrency(ctx, head.Hash(), blobBaseFee, args.FeeCurrency)
blobBaseFee, err = b.ConvertToCurrency(
ctx,
rpc.BlockNumberOrHashWithHash(head.Hash(), false),
blobBaseFee,
args.FeeCurrency,
)
if err != nil {
return fmt.Errorf("can't convert blob-fee to fee-currency: %w", err)
}
Expand All @@ -295,7 +305,12 @@ func (args *TransactionArgs) setLondonFeeDefaults(ctx context.Context, head *typ
return err
}
if args.IsFeeCurrencyDenominated() {
tip, err = b.ConvertToCurrency(ctx, head.Hash(), tip, args.FeeCurrency)
tip, err = b.ConvertToCurrency(
ctx,
rpc.BlockNumberOrHashWithHash(head.Hash(), false),
tip,
args.FeeCurrency,
)
if err != nil {
return fmt.Errorf("can't convert suggested gasTipCap to fee-currency: %w", err)
}
Expand All @@ -310,7 +325,12 @@ func (args *TransactionArgs) setLondonFeeDefaults(ctx context.Context, head *typ
baseFee := head.BaseFee
if args.IsFeeCurrencyDenominated() {
var err error
baseFee, err = b.ConvertToCurrency(ctx, head.Hash(), baseFee, args.FeeCurrency)
baseFee, err = b.ConvertToCurrency(
ctx,
rpc.BlockNumberOrHashWithHash(head.Hash(), false),
baseFee,
args.FeeCurrency,
)
if err != nil {
return fmt.Errorf("can't convert base-fee to fee-currency: %w", err)
}
Expand Down
8 changes: 4 additions & 4 deletions internal/ethapi/transaction_args_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,23 +264,23 @@ func newCeloBackendMock() *celoBackendMock {
}
}

func (c *celoBackendMock) GetFeeBalance(ctx context.Context, atBlock common.Hash, account common.Address, feeCurrency *common.Address) (*big.Int, error) {
func (c *celoBackendMock) GetFeeBalance(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, account common.Address, feeCurrency *common.Address) (*big.Int, error) {
// Celo specific backend features are currently not tested
return nil, errCeloNotImplemented
}

func (c *celoBackendMock) GetExchangeRates(ctx context.Context, atBlock common.Hash) (common.ExchangeRates, error) {
func (c *celoBackendMock) GetExchangeRates(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash) (common.ExchangeRates, error) {
var er common.ExchangeRates
// Celo specific backend features are currently not tested
return er, errCeloNotImplemented
}

func (c *celoBackendMock) ConvertToCurrency(ctx context.Context, atBlock common.Hash, value *big.Int, fromFeeCurrency *common.Address) (*big.Int, error) {
func (c *celoBackendMock) ConvertToCurrency(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, fromFeeCurrency *common.Address) (*big.Int, error) {
// Celo specific backend features are currently not tested
return nil, errCeloNotImplemented
}

func (c *celoBackendMock) ConvertToGold(ctx context.Context, atBlock common.Hash, value *big.Int, toFeeCurrency *common.Address) (*big.Int, error) {
func (c *celoBackendMock) ConvertToGold(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, toFeeCurrency *common.Address) (*big.Int, error) {
// Celo specific backend features are currently not tested
return nil, errCeloNotImplemented
}
Expand Down

0 comments on commit 74cfc8c

Please sign in to comment.