Skip to content
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.

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

EvmFeeEstimator return Optimistic Rollup's L1BaseFee #10557

Merged
merged 15 commits into from
Sep 16, 2023
151 changes: 151 additions & 0 deletions core/chains/evm/gas/chainoracles/l1_gas_price_oracle.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package chainoracles
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Just wondering, if this name is appropriate.
This seems to be a package for Rollups.
Could rollups be a better name?


import (
"context"
"fmt"
"math/big"
"sync"
"time"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common"

"github.com/smartcontractkit/chainlink/v2/core/assets"
evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
"github.com/smartcontractkit/chainlink/v2/core/config"
"github.com/smartcontractkit/chainlink/v2/core/logger"
"github.com/smartcontractkit/chainlink/v2/core/utils"
)

type OracleType string

// Reads L2-specific precompiles and caches the l1GasPrice set by the L2.
type l1GasPriceOracle struct {
client evmclient.Client
pollPeriod time.Duration
logger logger.Logger
address string
selector string

l1GasPriceMu sync.RWMutex
l1GasPrice *assets.Wei

chInitialised chan struct{}
chStop utils.StopChan
chDone chan struct{}
utils.StartStopOnce
}

const (
// ArbGasInfoAddress is the address of the "Precompiled contract that exists in every Arbitrum chain."
// https://github.com/OffchainLabs/nitro/blob/f7645453cfc77bf3e3644ea1ac031eff629df325/contracts/src/precompiles/ArbGasInfo.sol
ArbGasInfoAddress = "0x000000000000000000000000000000000000006C"
// ArbGasInfo_getL1BaseFeeEstimate is the a hex encoded call to:
// `function getL1BaseFeeEstimate() external view returns (uint256);`
ArbGasInfo_getL1BaseFeeEstimate = "f5d6ded7"

// GasOracleAddress is the address of the "Precompiled contract that exists in every OP stack chain."
OPGasOracleAddress = "0x420000000000000000000000000000000000000F"
// GasOracle_l1BaseFee is the a hex encoded call to:
// `function l1BaseFee() external view returns (uint256);`
OPGasOracle_l1BaseFee = "519b4bd3"

// Interval at which to poll for L1BaseFee. A good starting point is the L1 block time.
PollPeriod = 12 * time.Second
)

func NewL1GasPriceOracle(lggr logger.Logger, ethClient evmclient.Client, chainType config.ChainType) L1Oracle {
var address, selector string
switch chainType {
case config.ChainArbitrum:
address = ArbGasInfoAddress
selector = ArbGasInfo_getL1BaseFeeEstimate
case config.ChainOptimismBedrock:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since Base also uses OptimismBedorck chaintype, can we confirm the address and the hex code are the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

address = OPGasOracleAddress
selector = OPGasOracle_l1BaseFee
default:
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are calling this NewL1GasPriceOracle() only for chains that support it, the default case here should just call panic().
This will help surface up a misconfig or error faster.

}

return &l1GasPriceOracle{
client: ethClient,
pollPeriod: PollPeriod,
logger: lggr.Named(fmt.Sprintf("%d L1GasPriceOracle", chainType)),

Check failure on line 73 in core/chains/evm/gas/chainoracles/l1_gas_price_oracle.go

View workflow job for this annotation

GitHub Actions / lint

printf: fmt.Sprintf format %d has arg chainType of wrong type github.com/smartcontractkit/chainlink/v2/core/config.ChainType (govet)
address: address,
selector: selector,
}
}

func (o *l1GasPriceOracle) Name() string {
return o.logger.Name()
}

func (o *l1GasPriceOracle) Start(ctx context.Context) error {
return o.StartOnce(o.Name(), func() error {
go o.run()
<-o.chInitialised
return nil
})
}
func (o *l1GasPriceOracle) Close() error {
return o.StopOnce(o.Name(), func() (err error) {
close(o.chStop)
<-o.chDone
return
})
}

func (o *l1GasPriceOracle) Ready() error { return o.StartStopOnce.Ready() }

func (o *l1GasPriceOracle) HealthReport() map[string]error {
return map[string]error{o.Name(): o.StartStopOnce.Healthy()}
}

func (o *l1GasPriceOracle) run() {
defer close(o.chDone)

t := o.refresh()
close(o.chInitialised)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of from here, close this from within refresh(), once we successfully are able to set l1GasPrice there.
This way, this component is in started mode only after it fetches a valid l1GasPrice.

Copy link
Contributor Author

@matYang matYang Sep 14, 2023

Choose a reason for hiding this comment

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

The Start() call is blocked on the chInitialised channel, so we need to close it regardless of fetching prices (refresh()) was successful or not right, closing it once here makes sense to me. This pattern is same in L2Suggested and Arbitrum estimators.


for {
select {
case <-o.chStop:
return
case <-t.C:
t = o.refresh()
}
}
}

func (o *l1GasPriceOracle) refresh() (t *time.Timer) {
t = time.NewTimer(utils.WithJitter(o.pollPeriod))

ctx, cancel := o.chStop.CtxCancel(evmclient.ContextWithDefaultTimeout())
defer cancel()

precompile := common.HexToAddress(o.address)
b, err := o.client.CallContract(ctx, ethereum.CallMsg{
To: &precompile,
Data: common.Hex2Bytes(o.selector),
}, big.NewInt(-1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be nil instead. Not sure why on Arbitrum Estimator is also -1

if err != nil {
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Add an error log here too

}

if len(b) != 32 { // returns uint256;
o.logger.Warnf("return data length (%d) different than expected (%d)", len(b), 32)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a critical error. It indicates the response format has changed.

return
}
price := new(big.Int).SetBytes(b)

o.l1GasPriceMu.Lock()
defer o.l1GasPriceMu.Unlock()
o.l1GasPrice = assets.NewWei(price)
return
}

func (o *l1GasPriceOracle) L1GasPrice(_ context.Context) (*assets.Wei, error) {
o.l1GasPriceMu.RLock()
defer o.l1GasPriceMu.RUnlock()
return o.l1GasPrice, nil
}
129 changes: 129 additions & 0 deletions core/chains/evm/gas/chainoracles/mocks/l1_oracle.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions core/chains/evm/gas/chainoracles/models.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package chainoracles

import (
"context"

"github.com/smartcontractkit/chainlink/v2/core/assets"
"github.com/smartcontractkit/chainlink/v2/core/services"
)

// L1Oracle provides interface for fetching L1-specific fee components if the chain is an L2.
// For example, on Optimistic Rollups, this oracle can return rollup-specific l1BaseFee
//
//go:generate mockery --quiet --name L1Oracle --output ./mocks/ --case=underscore
type L1Oracle interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about renaming to UnderlyingL1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer L1Oracle since it is not actually calling the underlying L1, it is fetching from the L1 oracle contract provided by the L2

services.ServiceCtx

L1GasPrice(ctx context.Context) (*assets.Wei, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the interface itself is named L1..., this function doesn't need to repeat it. Just say GasPrice()

}
1 change: 1 addition & 0 deletions core/chains/evm/gas/fixed_price_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/pkg/errors"

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: revert this change, and keep this file untouched

commonfee "github.com/smartcontractkit/chainlink/v2/common/fee"
feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types"
"github.com/smartcontractkit/chainlink/v2/core/assets"
Expand Down
18 changes: 18 additions & 0 deletions core/chains/evm/gas/mocks/evm_fee_estimator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading