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.

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

Change PluginMedian Interface #166

Closed
wants to merge 21 commits into from
Closed

Change PluginMedian Interface #166

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f56670e
change interface
augustbleeds Sep 5, 2023
3e033ff
change signature for median factory
augustbleeds Sep 5, 2023
f8416cd
Merge branch 'main' into augustus.gas-prices-changes
augustbleeds Sep 6, 2023
c988207
serve gasPrice over grpc
augustbleeds Sep 8, 2023
c7bc6d8
update protobuf
augustbleeds Sep 8, 2023
0aa1d2d
add tests
augustbleeds Sep 8, 2023
7e621d3
Merge branch 'main' into augustus.gas-prices-changes
augustbleeds Sep 8, 2023
e51f862
lint fix
augustbleeds Sep 18, 2023
c604ab9
change to noop data source
augustbleeds Jan 12, 2024
852340c
merge main
augustbleeds Jan 12, 2024
cde3993
handle noop
augustbleeds Jan 12, 2024
02e97ec
use nil instead of err to rep noop
augustbleeds Jan 17, 2024
0837829
refactor tests
augustbleeds Jan 22, 2024
7c8f64f
Merge branch 'main' into augustus.gas-prices-changes
augustbleeds Jan 22, 2024
cb35a68
change noop to return 0
augustbleeds Feb 21, 2024
1e2185e
merge main
augustbleeds Feb 21, 2024
d57e9a8
try both static and noop
augustbleeds Feb 21, 2024
c9277a3
fix test
augustbleeds Feb 22, 2024
3220036
Merge branch 'main' into augustus.gas-prices-changes
augustbleeds Feb 22, 2024
9346ac9
point to new libocr
augustbleeds Feb 22, 2024
b6579c9
update libocr
augustbleeds Feb 22, 2024
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 go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace github.com/smartcontractkit/libocr => github.com/augustbleeds/libocr v0.0.0-20230901193648-78ff9454fecd
Copy link
Contributor Author

Choose a reason for hiding this comment

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

temp change: will get removed and bumped when libocr gets updated


replace (
// until merged upstream: https://github.com/hashicorp/go-plugin/pull/257
github.com/hashicorp/go-plugin => github.com/smartcontractkit/go-plugin v0.0.0-20230605132010-0f4d515d1472
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ github.com/actgardner/gogen-avro/v10 v10.1.0/go.mod h1:o+ybmVjEa27AAr35FRqU98DJu
github.com/actgardner/gogen-avro/v10 v10.2.1/go.mod h1:QUhjeHPchheYmMDni/Nx7VB0RsT/ee8YIgGY/xpEQgQ=
github.com/actgardner/gogen-avro/v9 v9.1.0/go.mod h1:nyTj6wPqDJoxM3qdnjcLv+EnMDSDFqE0qDpva2QRmKc=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/augustbleeds/libocr v0.0.0-20230901193648-78ff9454fecd h1:Iw9NqPsR8Oc71t+k3rjt0o2X2Imz4ZciluAlvb6lX7w=
github.com/augustbleeds/libocr v0.0.0-20230901193648-78ff9454fecd/go.mod h1:2lyRkw/qLQgUWlrWWmq5nj0y90rWeO6Y+v+fCakRgb0=
github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A=
github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
Expand Down Expand Up @@ -254,8 +256,6 @@ github.com/smartcontractkit/go-plugin v0.0.0-20230605132010-0f4d515d1472 h1:x3kN
github.com/smartcontractkit/go-plugin v0.0.0-20230605132010-0f4d515d1472/go.mod h1:6/1TEzT0eQznvI/gV2CM29DLSkAK/e58mUWKVsPaph0=
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-20230802221916-2271752fa829 h1:fzefK1SzoRSHzZduOCzIJ2kmkBMPKwIf3FgeBlw7Jjk=
github.com/smartcontractkit/libocr v0.0.0-20230802221916-2271752fa829/go.mod h1:2lyRkw/qLQgUWlrWWmq5nj0y90rWeO6Y+v+fCakRgb0=
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 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
Expand Down
6 changes: 4 additions & 2 deletions pkg/loop/internal/broker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ func (b *brokerExt) serve(name string, server *grpc.Server, deps ...resource) (u

func (b *brokerExt) closeAll(deps ...resource) {
for _, d := range deps {
if err := d.Close(); err != nil {
b.Logger.Error(fmt.Sprintf("Error closing %s", d.name), "err", err)
if d.Closer != nil {
Copy link
Contributor Author

@augustbleeds augustbleeds Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

handle case when the resource is declared but not initialized

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 This doesn't feel like it should be necessary, and I wonder if this will hide incorrect instantiations in the future. How come you had to add this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I guess it's due to this: https://github.com/smartcontractkit/chainlink-relay/pull/166/files#diff-28ea9d6d17fb7499fbbd0f83828a8335ac28eacff629256d75b261bdb2ee123aR147?

if err := d.Close(); err != nil {
b.Logger.Error(fmt.Sprintf("Error closing %s", d.name), "err", err)
}
}
}
}
Expand Down
36 changes: 31 additions & 5 deletions pkg/loop/internal/median.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func NewPluginMedianClient(broker Broker, brokerCfg BrokerConfig, conn *grpc.Cli
return &PluginMedianClient{pluginClient: pc, median: pb.NewPluginMedianClient(pc), serviceClient: newServiceClient(pc.brokerExt, pc)}
}

func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoin median.DataSource, errorLog types.ErrorLog) (types.ReportingPluginFactory, error) {
func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider types.MedianProvider, dataSource, juelsPerFeeCoin, gasPrice median.DataSource, errorLog types.ErrorLog) (types.ReportingPluginFactory, error) {
cc := m.newClientConn("MedianPluginFactory", func(ctx context.Context) (id uint32, deps resources, err error) {
dataSourceID, dsRes, err := m.serveNew("DataSource", func(s *grpc.Server) {
pb.RegisterDataSourceServer(s, &dataSourceServer{impl: dataSource})
Expand All @@ -53,6 +53,18 @@ func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider type
}
deps.Add(juelsPerFeeCoinDataSourceRes)

var gasPriceDataSourceID uint32
if gasPrice != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gasPrice may be nil if none is required for the chain

var gasPriceDataSourceRes resource
gasPriceDataSourceID, gasPriceDataSourceRes, err = m.serveNew("GasPriceDataSource", func(s *grpc.Server) {
pb.RegisterDataSourceServer(s, &dataSourceServer{impl: gasPrice})
})
if err != nil {
return 0, nil, err
}
deps.Add(gasPriceDataSourceRes)
}

var (
providerID uint32
providerRes resource
Expand Down Expand Up @@ -87,6 +99,7 @@ func (m *PluginMedianClient) NewMedianFactory(ctx context.Context, provider type
MedianProviderID: providerID,
DataSourceID: dataSourceID,
JuelsPerFeeCoinDataSourceID: juelsPerFeeCoinDataSourceID,
GasPriceDataSourceID: gasPriceDataSourceID,
ErrorLogID: errorLogID,
})
if err != nil {
Expand Down Expand Up @@ -131,25 +144,37 @@ func (m *pluginMedianServer) NewMedianFactory(ctx context.Context, request *pb.N
juelsRes := resource{juelsConn, "JuelsPerFeeCoinDataSource"}
juelsPerFeeCoin := newDataSourceClient(juelsConn)

var gasPriceRes resource
var gasPrice *dataSourceClient
if request.GasPriceDataSourceID != 0 {
gasPriceConn, err := m.dial(request.GasPriceDataSourceID)
if err != nil {
m.closeAll(dsRes, juelsRes)
return nil, ErrConnDial{Name: "GasPriceDataSource", ID: request.GasPriceDataSourceID, Err: err}
}
gasPriceRes = resource{gasPriceConn, "GasPriceDataSource"}
gasPrice = newDataSourceClient(gasPriceConn)
}

providerConn, err := m.dial(request.MedianProviderID)
if err != nil {
m.closeAll(dsRes, juelsRes)
m.closeAll(dsRes, juelsRes, gasPriceRes)
return nil, ErrConnDial{Name: "MedianProvider", ID: request.MedianProviderID, Err: err}
}
providerRes := resource{providerConn, "MedianProvider"}
provider := newMedianProviderClient(m.brokerExt, providerConn)

errorLogConn, err := m.dial(request.ErrorLogID)
if err != nil {
m.closeAll(dsRes, juelsRes, providerRes)
m.closeAll(dsRes, juelsRes, gasPriceRes, providerRes)
return nil, ErrConnDial{Name: "ErrorLog", ID: request.ErrorLogID, Err: err}
}
errorLogRes := resource{errorLogConn, "ErrorLog"}
errorLog := newErrorLogClient(errorLogConn)

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

Expand Down Expand Up @@ -222,6 +247,7 @@ func (r *reportCodecClient) BuildReport(observations []median.ParsedAttributedOb
Value: pb.NewBigIntFromInt(o.Value),
JulesPerFeeCoin: pb.NewBigIntFromInt(o.JuelsPerFeeCoin),
Observer: uint32(o.Observer),
GasPrice: pb.NewBigIntFromInt(o.GasPrice),
})
}
var reply *pb.BuildReportReply
Expand Down
Loading
Loading