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

BCI-1426: Gas Price Subunits Changes #509

Merged
merged 9 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
golang 1.21.4
golangci-lint 1.55.2
mockery 2.38.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this functional?
We have a make command that installs directly: https://github.com/smartcontractkit/chainlink-common/blob/main/Makefile#L17-L18

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah it works , i use asdf locally so I wanted to install this. it's the same version as the makefile

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
github.com/prometheus/client_golang v1.17.0
github.com/riferrei/srclient v0.5.4
github.com/shopspring/decimal v1.3.1
github.com/smartcontractkit/libocr v0.0.0-20240326191951-2bbe9382d052
github.com/smartcontractkit/libocr v0.0.0-20240419185742-fd3cab206b2c
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.45.0
go.opentelemetry.io/otel v1.19.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ github.com/smartcontractkit/go-plugin v0.0.0-20240208201424-b3b91517de16 h1:TFe+
github.com/smartcontractkit/go-plugin v0.0.0-20240208201424-b3b91517de16/go.mod h1:lBS5MtSSBZk0SHc66KACcjjlU6WzEVP/8pwz68aMkCI=
github.com/smartcontractkit/grpc-proxy v0.0.0-20230731113816-f1be6620749f h1:hgJif132UCdjo8u43i7iPN1/MFnu49hv7lFGFftCHKU=
github.com/smartcontractkit/grpc-proxy v0.0.0-20230731113816-f1be6620749f/go.mod h1:MvMXoufZAtqExNexqi4cjrNYE9MefKddKylxjS+//n0=
github.com/smartcontractkit/libocr v0.0.0-20240326191951-2bbe9382d052 h1:1WFjrrVrWoQ9UpVMh7Mx4jDpzhmo1h8hFUKd9awIhIU=
github.com/smartcontractkit/libocr v0.0.0-20240326191951-2bbe9382d052/go.mod h1:SJEZCHgMCAzzBvo9vMV2DQ9onfEcIJCYSViyP4JI6c4=
github.com/smartcontractkit/libocr v0.0.0-20240419185742-fd3cab206b2c h1:lIyMbTaF2H0Q71vkwZHX/Ew4KF2BxiKhqEXwF8rn+KI=
github.com/smartcontractkit/libocr v0.0.0-20240419185742-fd3cab206b2c/go.mod h1:fb1ZDVXACvu4frX3APHZaEBp0xi1DIm34DcA0CwTsZM=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
Expand Down
380 changes: 202 additions & 178 deletions pkg/loop/internal/pb/median.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions pkg/loop/internal/pb/median.proto
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ message NewMedianFactoryRequest {
uint32 dataSourceID = 2;
uint32 juelsPerFeeCoinDataSourceID = 3;
uint32 errorLogID = 4;
uint32 gasPriceSubunitsDataSourceID = 5;
}

// NewMedianFactoryRequest has return arguments for [github.com/smartcontractkit/chainlink-common/pkg/loop.Relayer.NewMedianFactory].
Expand Down Expand Up @@ -47,6 +48,7 @@ message ParsedAttributedObservation {
BigInt value = 2;
BigInt julesPerFeeCoin = 3;
uint32 observer = 4; // uint8
BigInt gasPriceSubunits = 5;
}

// BuildReportRequest has arguments for [github.com/smartcontractkit/libocr/offchainreporting2/reportingplugin/median.ReportCodec.BuildReport].
Expand Down
20 changes: 11 additions & 9 deletions pkg/loop/internal/relayer/pluginprovider/ext/median/median.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,11 @@ func (r *reportCodecClient) BuildReport(observations []median.ParsedAttributedOb
var req pb.BuildReportRequest
for _, o := range observations {
req.Observations = append(req.Observations, &pb.ParsedAttributedObservation{
Timestamp: o.Timestamp,
Value: pb.NewBigIntFromInt(o.Value),
JulesPerFeeCoin: pb.NewBigIntFromInt(o.JuelsPerFeeCoin),
Observer: uint32(o.Observer),
Timestamp: o.Timestamp,
Value: pb.NewBigIntFromInt(o.Value),
JulesPerFeeCoin: pb.NewBigIntFromInt(o.JuelsPerFeeCoin),
GasPriceSubunits: pb.NewBigIntFromInt(o.GasPriceSubunits),
Observer: uint32(o.Observer),
})
}
var reply *pb.BuildReportReply
Expand Down Expand Up @@ -138,15 +139,16 @@ type reportCodecServer struct {
func (r *reportCodecServer) BuildReport(ctx context.Context, request *pb.BuildReportRequest) (*pb.BuildReportReply, error) {
var obs []median.ParsedAttributedObservation
for _, o := range request.Observations {
val, jpfc := o.Value.Int(), o.JulesPerFeeCoin.Int()
val, jpfc, gpsu := o.Value.Int(), o.JulesPerFeeCoin.Int(), o.GasPriceSubunits.Int()
if o.Observer > math.MaxUint8 {
return nil, fmt.Errorf("expected uint8 Observer (max %d) but got %d", math.MaxUint8, o.Observer)
}
obs = append(obs, median.ParsedAttributedObservation{
Timestamp: o.Timestamp,
Value: val,
JuelsPerFeeCoin: jpfc,
Observer: commontypes.OracleID(o.Observer),
Timestamp: o.Timestamp,
Value: val,
JuelsPerFeeCoin: jpfc,
GasPriceSubunits: gpsu,
Observer: commontypes.OracleID(o.Observer),
})
}
report, err := r.impl.BuildReport(obs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ var (
Value: juelsPerFeeCoin,
},
}

GasPriceSubunitsDataSource = staticDataSource{
staticDataSourceConfig{
ReportContext: reportContext,
Value: gasPriceSubunits,
},
}
)

func (s staticDataSource) Observe(ctx context.Context, timestamp types.ReportTimestamp) (*big.Int, error) {
Expand All @@ -46,13 +53,34 @@ func (s staticDataSource) Observe(ctx context.Context, timestamp types.ReportTim
return s.Value, nil
}

type CompareError struct {
Got *big.Int
Expected *big.Int
}

func (e *CompareError) Error() string {
return fmt.Sprintf("expected Value %s but got %s", e.Expected, e.Got)
}

func (e *CompareError) GotZero() bool {
return e.Got.Uint64() == 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit/ Could tighten this up a bit:

Suggested change
return e.Got.Uint64() == 0
return e.Got.IsUint64() && e.Got.Uint64() == 0

}

func (s staticDataSource) Evaluate(ctx context.Context, ds median.DataSource) error {
gotVal, err := ds.Observe(ctx, s.ReportContext.ReportTimestamp)
if err != nil {
return fmt.Errorf("failed to observe dataSource: %w", err)
}
if gotVal.Cmp(s.Value) != 0 {
return fmt.Errorf("expected Value %s but got %s", value, gotVal)
return &CompareError{Got: gotVal, Expected: s.Value}
}
return nil
}

// Only to be used for testing
type ZeroDataSource struct {
}

func (s *ZeroDataSource) Observe(ctx context.Context, _ types.ReportTimestamp) (*big.Int, error) {
return big.NewInt(0), nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package median_test
import (
"bytes"
"context"
"errors"
"fmt"
"math/big"
"reflect"
Expand Down Expand Up @@ -34,7 +35,16 @@ type PluginMedianTest struct {
func (m PluginMedianTest) TestPluginMedian(t *testing.T, p core.PluginMedian) {
t.Run("PluginMedian", func(t *testing.T) {
ctx := tests.Context(t)
factory, err := p.NewMedianFactory(ctx, m.MedianProvider, DataSource, JuelsPerFeeCoinDataSource, &errorlogtest.ErrorLog)
factory, err := p.NewMedianFactory(ctx, m.MedianProvider, DataSource, JuelsPerFeeCoinDataSource, GasPriceSubunitsDataSource, &errorlogtest.ErrorLog)
require.NoError(t, err)

ReportingPluginFactory(t, factory)
})

// when gasPriceSubunitsDataSource is meant to trigger a no-op
t.Run("PluginMedian (Zero GasPriceSubunitsDataSource)", func(t *testing.T) {
ctx := tests.Context(t)
factory, err := p.NewMedianFactory(ctx, m.MedianProvider, DataSource, JuelsPerFeeCoinDataSource, &ZeroDataSource{}, &errorlogtest.ErrorLog)
require.NoError(t, err)

ReportingPluginFactory(t, factory)
Expand All @@ -61,10 +71,11 @@ func ReportingPluginFactory(t *testing.T, factory types.ReportingPluginFactory)
}

type staticPluginMedianConfig struct {
provider staticMedianProvider
dataSource staticDataSource
juelsPerFeeCoinDataSource staticDataSource
errorLog testtypes.ErrorLogEvaluator
provider staticMedianProvider
dataSource staticDataSource
juelsPerFeeCoinDataSource staticDataSource
gasPriceSubunitsDataSource staticDataSource
errorLog testtypes.ErrorLogEvaluator
}

type staticMedianFactoryServer struct {
Expand All @@ -73,7 +84,7 @@ type staticMedianFactoryServer struct {

var _ core.PluginMedian = staticMedianFactoryServer{}

func (s staticMedianFactoryServer) NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoinDataSource median.DataSource, errorLog core.ErrorLog) (types.ReportingPluginFactory, error) {
func (s staticMedianFactoryServer) NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoinDataSource, gasPriceSubunitsDataSource median.DataSource, errorLog core.ErrorLog) (types.ReportingPluginFactory, error) {
// the provider may be a grpc client, so we can't compare it directly
// but in all of these static tests, the implementation of the provider is expected
// to be the same static implementation, so we can compare the expected values
Expand All @@ -93,6 +104,17 @@ func (s staticMedianFactoryServer) NewMedianFactory(ctx context.Context, provide
return nil, fmt.Errorf("NewMedianFactory: juelsPerFeeCoinDataSource does not equal a static test juels per fee coin data source implementation: %w", err)
}

err = s.gasPriceSubunitsDataSource.Evaluate(ctx, gasPriceSubunitsDataSource)

if err != nil {
var compareError *CompareError
isCompareError := errors.As(err, &compareError)
// allow 0 as valid data source value with the same staticMedianFactoryServer (because it is only defined once as a global var for all tests)
if !(isCompareError && compareError.GotZero()) {
return nil, fmt.Errorf("NewMedianFactory: gasPriceSubunitsDataSource does not equal a static gas price subunits data source implementation: %w", err)
}
}

if err := errorLog.SaveError(ctx, "an error"); err != nil {
return nil, fmt.Errorf("failed to save error: %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ const (
var (
MedianFactoryServer = staticMedianFactoryServer{
staticPluginMedianConfig: staticPluginMedianConfig{
provider: MedianProvider,
dataSource: DataSource,
juelsPerFeeCoinDataSource: JuelsPerFeeCoinDataSource,
errorLog: errorlogtest.ErrorLog,
provider: MedianProvider,
dataSource: DataSource,
juelsPerFeeCoinDataSource: JuelsPerFeeCoinDataSource,
gasPriceSubunitsDataSource: GasPriceSubunitsDataSource,
errorLog: errorlogtest.ErrorLog,
},
}

Expand Down Expand Up @@ -58,10 +59,11 @@ var (
var (
encodedOnchainConfig = []byte{5: 11}
juelsPerFeeCoin = big.NewInt(1234)
gasPriceSubunits = big.NewInt(777)
onchainConfig = median.OnchainConfig{Min: big.NewInt(-12), Max: big.NewInt(1234567890987654321)}
medianValue = big.NewInt(-1042)

pobs = []median.ParsedAttributedObservation{{Timestamp: 123, Value: big.NewInt(31), JuelsPerFeeCoin: big.NewInt(54), Observer: commontypes.OracleID(99)}}
pobs = []median.ParsedAttributedObservation{{Timestamp: 123, Value: big.NewInt(31), JuelsPerFeeCoin: big.NewInt(54), GasPriceSubunits: big.NewInt(77), Observer: commontypes.OracleID(99)}}

report = libocr.Report{42: 101}
reportContext = libocr.ReportContext{
Expand Down
37 changes: 27 additions & 10 deletions pkg/loop/internal/reportingplugin/median/median.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func NewPluginMedianClient(broker net.Broker, brokerCfg net.BrokerConfig, conn *
return &PluginMedianClient{PluginClient: pc, median: pb.NewPluginMedianClient(pc), ServiceClient: goplugin.NewServiceClient(pc.BrokerExt, pc)}
}

func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoin median.DataSource, errorLog core.ErrorLog) (types.ReportingPluginFactory, error) {
func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoin, gasPriceSubunits median.DataSource, errorLog core.ErrorLog) (types.ReportingPluginFactory, error) {
cc := m.NewClientConn("MedianPluginFactory", func(ctx context.Context) (id uint32, deps net.Resources, err error) {
dataSourceID, dsRes, err := m.ServeNew("DataSource", func(s *grpc.Server) {
pb.RegisterDataSourceServer(s, newDataSourceServer(dataSource))
Expand All @@ -52,6 +52,14 @@ func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider type
}
deps.Add(juelsPerFeeCoinDataSourceRes)

gasPriceSubunitsDataSourceID, gasPriceSubunitsDataSourceRes, err := m.ServeNew("GasPriceSubunitsDataSource", func(s *grpc.Server) {
pb.RegisterDataSourceServer(s, newDataSourceServer(gasPriceSubunits))
})
if err != nil {
return 0, nil, err
}
deps.Add(gasPriceSubunitsDataSourceRes)

var (
providerID uint32
providerRes net.Resource
Expand All @@ -77,10 +85,11 @@ func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider type
deps.Add(errorLogRes)

reply, err := m.median.NewMedianFactory(ctx, &pb.NewMedianFactoryRequest{
MedianProviderID: providerID,
DataSourceID: dataSourceID,
JuelsPerFeeCoinDataSourceID: juelsPerFeeCoinDataSourceID,
ErrorLogID: errorLogID,
MedianProviderID: providerID,
DataSourceID: dataSourceID,
JuelsPerFeeCoinDataSourceID: juelsPerFeeCoinDataSourceID,
GasPriceSubunitsDataSourceID: gasPriceSubunitsDataSourceID,
ErrorLogID: errorLogID,
})
if err != nil {
return 0, nil, err
Expand Down Expand Up @@ -124,32 +133,40 @@ func (m *pluginMedianServer) NewMedianFactory(ctx context.Context, request *pb.N
juelsRes := net.Resource{Closer: juelsConn, Name: "JuelsPerFeeCoinDataSource"}
juelsPerFeeCoin := newDataSourceClient(juelsConn)

providerConn, err := m.Dial(request.MedianProviderID)
gasPriceSubunitsConn, err := m.Dial(request.GasPriceSubunitsDataSourceID)
if err != nil {
m.CloseAll(dsRes, juelsRes)
return nil, net.ErrConnDial{Name: "GasPriceSubunitsDataSource", ID: request.GasPriceSubunitsDataSourceID, Err: err}
}
gasPriceSubunitsRes := net.Resource{Closer: gasPriceSubunitsConn, Name: "GasPriceSubunitsDataSource"}
gasPriceSubunits := newDataSourceClient(gasPriceSubunitsConn)

providerConn, err := m.Dial(request.MedianProviderID)
if err != nil {
m.CloseAll(dsRes, juelsRes, gasPriceSubunitsRes)
return nil, net.ErrConnDial{Name: "MedianProvider", ID: request.MedianProviderID, Err: err}
}
providerRes := net.Resource{Closer: providerConn, Name: "MedianProvider"}
provider := medianprovider.NewProviderClient(m.BrokerExt, providerConn)

errorLogConn, err := m.Dial(request.ErrorLogID)
if err != nil {
m.CloseAll(dsRes, juelsRes, providerRes)
m.CloseAll(dsRes, juelsRes, gasPriceSubunitsRes, providerRes)
return nil, net.ErrConnDial{Name: "ErrorLog", ID: request.ErrorLogID, Err: err}
}
errorLogRes := net.Resource{Closer: errorLogConn, Name: "ErrorLog"}
errorLog := errorlog.NewClient(errorLogConn)

factory, err := m.impl.NewMedianFactory(ctx, provider, dataSource, juelsPerFeeCoin, errorLog)
factory, err := m.impl.NewMedianFactory(ctx, provider, dataSource, juelsPerFeeCoin, gasPriceSubunits, errorLog)
if err != nil {
m.CloseAll(dsRes, juelsRes, providerRes, errorLogRes)
m.CloseAll(dsRes, juelsRes, gasPriceSubunitsRes, providerRes, errorLogRes)
return nil, err
}

id, _, err := m.ServeNew("ReportingPluginProvider", func(s *grpc.Server) {
pb.RegisterServiceServer(s, &goplugin.ServiceServer{Srv: factory})
pb.RegisterReportingPluginFactoryServer(s, ocr2.NewReportingPluginFactoryServer(factory, m.BrokerExt))
}, dsRes, juelsRes, providerRes, errorLogRes)
}, dsRes, juelsRes, gasPriceSubunitsRes, providerRes, errorLogRes)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/loop/median_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ type MedianService struct {

// NewMedianService returns a new [*MedianService].
// cmd must return a new exec.Cmd each time it is called.
func NewMedianService(lggr logger.Logger, grpcOpts GRPCOpts, cmd func() *exec.Cmd, provider types.MedianProvider, dataSource, juelsPerFeeCoin median.DataSource, errorLog core.ErrorLog) *MedianService {
func NewMedianService(lggr logger.Logger, grpcOpts GRPCOpts, cmd func() *exec.Cmd, provider types.MedianProvider, dataSource, juelsPerFeeCoin, gasPriceSubunits median.DataSource, errorLog core.ErrorLog) *MedianService {
newService := func(ctx context.Context, instance any) (types.ReportingPluginFactory, error) {
plug, ok := instance.(core.PluginMedian)
if !ok {
return nil, fmt.Errorf("expected PluginMedian but got %T", instance)
}
return plug.NewMedianFactory(ctx, provider, dataSource, juelsPerFeeCoin, errorLog)
return plug.NewMedianFactory(ctx, provider, dataSource, juelsPerFeeCoin, gasPriceSubunits, errorLog)
}
stopCh := make(chan struct{})
lggr = logger.Named(lggr, "MedianService")
Expand Down
4 changes: 2 additions & 2 deletions pkg/loop/median_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestMedianService(t *testing.T) {

median := loop.NewMedianService(logger.Test(t), loop.GRPCOpts{}, func() *exec.Cmd {
return NewHelperProcessCommand(loop.PluginMedianName, false, 0)
}, mediantest.MedianProvider, mediantest.DataSource, mediantest.JuelsPerFeeCoinDataSource, errorlogtest.ErrorLog)
}, mediantest.MedianProvider, mediantest.DataSource, mediantest.JuelsPerFeeCoinDataSource, mediantest.GasPriceSubunitsDataSource, errorlogtest.ErrorLog)
hook := median.PluginService.XXXTestHook()
servicetest.Run(t, median)

Expand Down Expand Up @@ -56,7 +56,7 @@ func TestMedianService_recovery(t *testing.T) {
Limit: int(limit.Add(1)),
}
return h.New()
}, mediantest.MedianProvider, mediantest.DataSource, mediantest.JuelsPerFeeCoinDataSource, errorlogtest.ErrorLog)
}, mediantest.MedianProvider, mediantest.DataSource, mediantest.JuelsPerFeeCoinDataSource, mediantest.GasPriceSubunitsDataSource, errorlogtest.ErrorLog)
servicetest.Run(t, median)

reportingplugintest.RunFactory(t, median)
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/core/median.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ import (

type PluginMedian interface {
// NewMedianFactory returns a new ReportingPluginFactory. If provider implements GRPCClientConn, it can be forwarded efficiently via proxy.
NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoin median.DataSource, errorLog ErrorLog) (types.ReportingPluginFactory, error)
NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoin, gasPriceSubunits median.DataSource, errorLog ErrorLog) (types.ReportingPluginFactory, error)
}
Loading