-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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-3989][core] - CR methods err when service unstarted #14123
base: develop
Are you sure you want to change the base?
Changes from 54 commits
111057c
6f8ed23
0074a33
2132bf0
53adad8
bf588c7
2cce774
e8b366e
7fa91e6
ed83615
fa2d416
4b2e503
7184150
1e41db4
add59d8
db99f56
7fde790
83c2559
52e3313
b0c3305
46d0c84
deb2d30
8a7054d
2aa2ee3
c63bcc6
733e0e0
894835e
d7153ac
be1d2c8
46d7e32
4ea601d
19a0268
a0344e4
5f45f15
482bb8b
5c93499
0e8f75f
2eb9b0d
6381bcd
f8d149d
160ea54
0272eec
d945744
017380c
7448064
6fc175f
2a236da
e42f6dd
3bff641
917add2
732f4c0
ca2124d
19ac700
f13379f
ec4dcbd
3684bde
eaa8149
e06b1d1
d6e2d43
cadbc0f
2715354
9b8be2f
8ddc56c
8095ff8
872fd3c
00044c6
e1792e9
76ed985
218f3cd
d8e1c2b
37c892c
3fcbbf6
56b0f5f
e569a69
36e4489
c83a850
40d2cc1
e96d20c
07c8f01
b22ad6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"chainlink": minor | ||
--- | ||
|
||
chainReader methods return err when called and service is not started yet. #internal |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
github.com/prometheus/client_golang v1.17.0 | ||
github.com/shopspring/decimal v1.4.0 | ||
github.com/smartcontractkit/chainlink-automation v1.0.4 | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828152114-571392f2833a | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828163211-dd26cd745d58 | ||
Check failure on line 25 in core/scripts/go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/smartcontractkit/chainlink/v2 v2.0.0-00010101000000-000000000000 | ||
github.com/smartcontractkit/libocr v0.0.0-20240717100443-f6226e09bee7 | ||
github.com/spf13/cobra v1.8.0 | ||
|
@@ -278,7 +278,7 @@ | |
github.com/smartcontractkit/chainlink-starknet/relayer v0.0.1-beta-test.0.20240709043547-03612098f799 // indirect | ||
github.com/smartcontractkit/tdh2/go/ocr2/decryptionplugin v0.0.0-20230906073235-9e478e5e19f1 // indirect | ||
github.com/smartcontractkit/tdh2/go/tdh2 v0.0.0-20230906073235-9e478e5e19f1 // indirect | ||
github.com/smartcontractkit/wsrpc v0.7.3 // indirect | ||
Check failure on line 281 in core/scripts/go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/spf13/afero v1.9.5 // indirect | ||
github.com/spf13/cast v1.6.0 // indirect | ||
github.com/spf13/jwalterweatherman v1.1.0 // indirect | ||
|
@@ -367,7 +367,7 @@ | |
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 | ||
|
||
// until merged upstream: https://github.com/hashicorp/go-plugin/pull/257 | ||
github.com/hashicorp/go-plugin => github.com/smartcontractkit/go-plugin v0.0.0-20240208201424-b3b91517de16 | ||
Check failure on line 370 in core/scripts/go.mod GitHub Actions / Validate go.mod dependencies
|
||
|
||
// until merged upstream: https://github.com/mwitkow/grpc-proxy/pull/69 | ||
github.com/mwitkow/grpc-proxy => github.com/smartcontractkit/grpc-proxy v0.0.0-20230731113816-f1be6620749f | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ | |
github.com/smartcontractkit/chain-selectors v1.0.21 | ||
github.com/smartcontractkit/chainlink-automation v1.0.4 | ||
github.com/smartcontractkit/chainlink-ccip v0.0.0-20240806144315-04ac101e9c95 | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828152114-571392f2833a | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828163211-dd26cd745d58 | ||
Check failure on line 78 in go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/smartcontractkit/chainlink-cosmos v0.4.1-0.20240710121324-3ed288aa9b45 | ||
github.com/smartcontractkit/chainlink-data-streams v0.0.0-20240820130645-cf4b159fbba2 | ||
github.com/smartcontractkit/chainlink-feeds v0.0.0-20240710170203-5b41615da827 | ||
|
@@ -84,7 +84,7 @@ | |
github.com/smartcontractkit/libocr v0.0.0-20240717100443-f6226e09bee7 | ||
github.com/smartcontractkit/tdh2/go/ocr2/decryptionplugin v0.0.0-20230906073235-9e478e5e19f1 | ||
github.com/smartcontractkit/tdh2/go/tdh2 v0.0.0-20230906073235-9e478e5e19f1 | ||
github.com/smartcontractkit/wsrpc v0.7.3 | ||
Check failure on line 87 in go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/spf13/cast v1.6.0 | ||
github.com/stretchr/testify v1.9.0 | ||
github.com/test-go/testify v1.1.4 | ||
|
@@ -353,7 +353,7 @@ | |
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 | ||
|
||
// until merged upstream: https://github.com/hashicorp/go-plugin/pull/257 | ||
github.com/hashicorp/go-plugin => github.com/smartcontractkit/go-plugin v0.0.0-20240208201424-b3b91517de16 | ||
Check failure on line 356 in go.mod GitHub Actions / Validate go.mod dependencies
|
||
|
||
// until nolag updates merged upstream | ||
github.com/mitchellh/mapstructure => github.com/nolag/mapstructure v1.5.2-0.20240625151721-90ea83a3f479 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,12 +36,12 @@ | |
github.com/smartcontractkit/ccip-owner-contracts v0.0.0-20240808195812-ae0378684685 | ||
github.com/smartcontractkit/chain-selectors v1.0.21 | ||
github.com/smartcontractkit/chainlink-automation v1.0.4 | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828152114-571392f2833a | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828163211-dd26cd745d58 | ||
Check failure on line 39 in integration-tests/go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/smartcontractkit/chainlink-testing-framework v1.34.10 | ||
github.com/smartcontractkit/chainlink-testing-framework/grafana v0.0.1 | ||
github.com/smartcontractkit/chainlink-testing-framework/havoc v0.0.0-20240822140612-df8e03c10dc1 | ||
github.com/smartcontractkit/chainlink-testing-framework/seth v1.2.1 | ||
github.com/smartcontractkit/chainlink-testing-framework/wasp v0.4.10 | ||
Check warning on line 44 in integration-tests/go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/smartcontractkit/chainlink/v2 v2.0.0-00010101000000-000000000000 | ||
github.com/smartcontractkit/libocr v0.0.0-20240717100443-f6226e09bee7 | ||
github.com/spf13/cobra v1.8.1 | ||
|
@@ -518,7 +518,7 @@ | |
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 | ||
|
||
// until merged upstream: https://github.com/hashicorp/go-plugin/pull/257 | ||
github.com/hashicorp/go-plugin => github.com/smartcontractkit/go-plugin v0.0.0-20240208201424-b3b91517de16 | ||
Check failure on line 521 in integration-tests/go.mod GitHub Actions / Validate go.mod dependencies
|
||
|
||
// until merged upstream: https://github.com/mwitkow/grpc-proxy/pull/69 | ||
github.com/mwitkow/grpc-proxy => github.com/smartcontractkit/grpc-proxy v0.0.0-20230731113816-f1be6620749f | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,10 @@ | |
github.com/rs/zerolog v1.33.0 | ||
github.com/slack-go/slack v0.12.2 | ||
github.com/smartcontractkit/chainlink-automation v1.0.4 | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828152114-571392f2833a | ||
github.com/smartcontractkit/chainlink-common v0.2.2-0.20240828163211-dd26cd745d58 | ||
Check failure on line 19 in integration-tests/load/go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/smartcontractkit/chainlink-testing-framework v1.34.10 | ||
github.com/smartcontractkit/chainlink-testing-framework/seth v1.2.1 | ||
github.com/smartcontractkit/chainlink-testing-framework/wasp v0.4.10 | ||
Check warning on line 22 in integration-tests/load/go.mod GitHub Actions / Validate go.mod dependencies
|
||
github.com/smartcontractkit/chainlink/integration-tests v0.0.0-20240214231432-4ad5eb95178c | ||
github.com/smartcontractkit/chainlink/v2 v2.9.0-beta0.0.20240216210048-da02459ddad8 | ||
github.com/smartcontractkit/libocr v0.0.0-20240717100443-f6226e09bee7 | ||
|
@@ -515,7 +515,7 @@ | |
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 | ||
|
||
// until merged upstream: https://github.com/hashicorp/go-plugin/pull/257 | ||
github.com/hashicorp/go-plugin => github.com/smartcontractkit/go-plugin v0.0.0-20240208201424-b3b91517de16 | ||
Check failure on line 518 in integration-tests/load/go.mod GitHub Actions / Validate go.mod dependencies
|
||
|
||
// until merged upstream: https://github.com/mwitkow/grpc-proxy/pull/69 | ||
github.com/mwitkow/grpc-proxy => github.com/smartcontractkit/grpc-proxy v0.0.0-20230731113816-f1be6620749f | ||
ilija42 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be started in
func (s *registrySyncer) Start(ctx context.Context) error {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (s *registrySyncer) Start(ctx context.Context) error
callsfunc (s *registrySyncer) syncLoop()
that callsfunc (s *registrySyncer) Sync(ctx context.Context, isInitialSync bool) error
which initializes the CR.I added these lines to start the CR service right after this initialization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it better to have the initialization here, rather than in
(*registrySyncer).Start
? Isn't that just contributing to this problem in the first place?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by
this problem
?There's a note in the
newReader
func that says why this is achieved this way:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these lines as we are now enforcing
CR
service to be inStarted
state before calling the methods. Thissyncer
was not starting the service previously and that's why we got some tests failing but now they are passing with this changeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If all dependencies are
Started
before anything that uses them, then there is no need to be defensive about checking internally ifStart
has been called. But I see that there is a good reason to defer in this case.