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

BCF-2654 chain reader get latest value evm poc WIP #11272

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a72f6e4
Implement skeleton interfaces, structs, & methods for ChainReader EVM…
reductionista Oct 18, 2023
d8bae2a
Read ChainReader config in from RelayConfig
reductionista Oct 19, 2023
e87c6dd
Use medianProviderWrapper instead of passing medianContract separately
reductionista Nov 7, 2023
36b8c95
Update relay & .tools-version
reductionista Nov 9, 2023
43bd0e7
Add chain_reader_test.go stub for relay tests
reductionista Oct 20, 2023
e824b63
Some minor PR suggestions
reductionista Oct 27, 2023
51fa7d2
Return error messages from newChainReader instead of nil on failure
reductionista Oct 27, 2023
60cd2b3
err.Error() -> err
reductionista Oct 30, 2023
895d21a
Fix "unimplemented method" bug
reductionista Oct 31, 2023
c6f4d66
Add chain reader config validation
ilija42 Oct 31, 2023
d4eb5af
Add chain reader config validation tests
ilija42 Oct 31, 2023
32a468c
minor fix in chain_reader_test.go
ilija42 Oct 31, 2023
efb3e09
Fix chainReader = nil bug
reductionista Nov 1, 2023
1b6e4ae
Restore medianContract to evm relay
reductionista Nov 1, 2023
f4dda80
Fix lint errors, re-run go mod tidy
reductionista Nov 2, 2023
187ee9a
Add config for chain reader median contract to cr validation testcases
ilija42 Nov 4, 2023
1b367ae
Add unimplemented Encode(), Decode(), GetMaxEncodingSize(), GetMaxDec…
reductionista Nov 8, 2023
f187123
goimports -local github.com/smartcontractkit/chainlink ./core/service…
reductionista Nov 8, 2023
5625c4a
Add ChainReader() method to mock provider for plugin test
reductionista Nov 8, 2023
7fce04a
Replace relay refs in go.mod files
reductionista Nov 9, 2023
5a983a2
Changes to chain reader constructor and add more tests
ilija42 Nov 13, 2023
acdcd1f
Add method to JSONCONFIG that allows us to marshal embedded json struct
ilija42 Nov 13, 2023
9393d07
Fix median contract response structs to match abi for easier decoding
ilija42 Nov 13, 2023
5e101de
Register lp filter for chain reader in median provider constructor
ilija42 Nov 13, 2023
f5f87c4
Add ChainReader GetLatestValue EVM implementation
ilija42 Nov 13, 2023
7b48083
Change integration tests ocr2 helpers to include chainReader bool flag
ilija42 Nov 13, 2023
62fe5e4
Add ocr2 basic with chainReader smoke test
ilija42 Nov 13, 2023
e7228ae
Refactor GetLatestValue to be more readable
ilija42 Nov 13, 2023
cee0973
Change usage of errors.Wrap() to fmt.Errorf()
ilija42 Nov 14, 2023
387ea20
ADD TODOS to chainReader GetLatestValue
ilija42 Nov 14, 2023
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: 1 addition & 1 deletion .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ jobs:
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
repository: smartcontractkit/chainlink-solana
ref: develop
ref: BCF-2612-ChainReader
fetch-depth: 0
path: solanapath
- name: Get long sha
Expand Down
2 changes: 1 addition & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
golang 1.21.1
golang 1.21.3
mockery 2.35.4
nodejs 16.16.0
postgres 13.3
Expand Down
2 changes: 1 addition & 1 deletion core/scripts/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/docker/docker v24.0.6+incompatible
github.com/docker/go-connections v0.4.0
github.com/ethereum/go-ethereum v1.12.0
github.com/google/go-cmp v0.5.9
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.3.1
github.com/joho/godotenv v1.4.0
github.com/manyminds/api2go v0.0.0-20171030193247-e7b693844a6f
Expand Down
3 changes: 2 additions & 1 deletion core/scripts/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,9 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
Expand Down
28 changes: 28 additions & 0 deletions core/services/job/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,34 @@ func (r JSONConfig) Bytes() []byte {
return b
}

// BytesWithPreservedJson returns raw bytes and properly marshals any potential json structure strings.
func (r JSONConfig) BytesWithPreservedJson() []byte {
var retCopy = make(JSONConfig, 0)
for key, value := range r {
copiedVal := value
// If the value is a json structure string, unmarshal it to preserve JSON structure
// e.g. instead of this {"key":"{\"nestedKey\":{\"nestedValue\":123}}"}
// we want this {"key":{"nestedKey":{"nestedValue":123}}},
if strValue, ok := copiedVal.(string); ok {
if isValidJSONStruct(strValue) {
var parsedValue interface{}
if err := json.Unmarshal([]byte(strValue), &parsedValue); err == nil {
copiedVal = parsedValue
}
}
}
retCopy[key] = copiedVal
}

b, _ := json.Marshal(retCopy)
return b
}

func isValidJSONStruct(s string) bool {
var js map[string]interface{}
return json.Unmarshal([]byte(s), &js) == nil
}

// Value returns this instance serialized for database storage.
func (r JSONConfig) Value() (driver.Value, error) {
return json.Marshal(r)
Expand Down
66 changes: 66 additions & 0 deletions core/services/job/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,69 @@ func TestOCR2OracleSpec_RelayIdentifier(t *testing.T) {
})
}
}

func TestJSONConfig_BytesWithPreservedJson(t *testing.T) {
type testCases struct {
name string
Input JSONConfig
Expected []byte
}
tests := []testCases{
{
name: "json",
Input: JSONConfig{
"key": "{\"nestedKey\": {\"nestedValue\":123}}",
},
// regular Bytes marshals to this: {"key":"{\"nestedKey\": {\"nestedValue\":123}}"}
Expected: []byte(`{"key":{"nestedKey":{"nestedValue":123}}}`),
},
{
name: "broken json gets treated as a regular string",
Input: JSONConfig{
"key": "2324{\"nes4tedKey\":\"nestedValue\"}",
},
Expected: []byte(`{"key":"2324{\"nes4tedKey\":\"nestedValue\"}"}`),
},
{
name: "number",
Input: JSONConfig{
"key": 1,
},
Expected: []byte(`{"key":1}`),
},
{
name: "string",
Input: JSONConfig{
"key": "abc",
},
Expected: []byte(`{"key":"abc"}`),
},
{
name: "string number stays string number",
Input: JSONConfig{
"key1": "1",
},
Expected: []byte(`{"key1":"1"}`),
},
{
name: "all together",
Input: JSONConfig{
"key1": "{\"nestedKey\": {\"nestedValue\":123}}",
"key2": "2324{\"key\":\"value\"}",
"key3": 1,
"key4": "abc",
},
Expected: []byte(`{"key1":{"nestedKey":{"nestedValue":123}},"key2":"2324{\"key\":\"value\"}","key3":1,"key4":"abc"}`),
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := tc.Input.BytesWithPreservedJson()
if !reflect.DeepEqual(result, tc.Expected) {
t.Errorf("Input: %v, BytesWithPreservedJson() returned unexpected result. Expected: %s, Got: %s", tc.Input, tc.Expected, result)
}
})

}
}
14 changes: 7 additions & 7 deletions core/services/ocr2/delegate.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ func (d *Delegate) cleanupEVM(jb job.Job, q pg.Queryer, relayID relay.ID) error
JobID: jb.ID,
ContractID: spec.ContractID,
New: false,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
}

relayFilters, err := evmrelay.FilterNamesFromRelayArgs(rargs)
Expand Down Expand Up @@ -552,7 +552,7 @@ func (d *Delegate) newServicesGenericPlugin(
JobID: spec.ID,
ContractID: spec.ContractID,
New: d.isNewlyCreatedJob,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
ProviderType: cconf.ProviderType,
}, types.PluginArgs{
TransmitterID: spec.TransmitterID.String,
Expand Down Expand Up @@ -664,7 +664,7 @@ func (d *Delegate) newServicesMercury(
JobID: jb.ID,
ContractID: spec.ContractID,
New: d.isNewlyCreatedJob,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
ProviderType: string(spec.PluginType),
}, types.PluginArgs{
TransmitterID: transmitterID,
Expand Down Expand Up @@ -785,7 +785,7 @@ func (d *Delegate) newServicesDKG(
JobID: jb.ID,
ContractID: spec.ContractID,
New: d.isNewlyCreatedJob,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
}, types.PluginArgs{
TransmitterID: spec.TransmitterID.String,
PluginConfig: spec.PluginConfig.Bytes(),
Expand Down Expand Up @@ -870,7 +870,7 @@ func (d *Delegate) newServicesOCR2VRF(
JobID: jb.ID,
ContractID: spec.ContractID,
New: d.isNewlyCreatedJob,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
}, types.PluginArgs{
TransmitterID: transmitterID,
PluginConfig: spec.PluginConfig.Bytes(),
Expand All @@ -884,7 +884,7 @@ func (d *Delegate) newServicesOCR2VRF(
ExternalJobID: jb.ExternalJobID,
JobID: jb.ID,
ContractID: cfg.DKGContractAddress,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
}, types.PluginArgs{
TransmitterID: transmitterID,
PluginConfig: spec.PluginConfig.Bytes(),
Expand Down Expand Up @@ -1327,7 +1327,7 @@ func (d *Delegate) newServicesOCR2Functions(
ExternalJobID: jb.ExternalJobID,
JobID: jb.ID,
ContractID: spec.ContractID,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
New: d.isNewlyCreatedJob,
},
types.PluginArgs{
Expand Down
1 change: 1 addition & 0 deletions core/services/ocr2/plugins/median/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func (p *Plugin) NewMedianFactory(ctx context.Context, provider types.MedianProv
var ctxVals loop.ContextValues
ctxVals.SetValues(ctx)
lggr := logger.With(p.Logger, ctxVals.Args()...)

factory := median.NumericalMedianFactory{
ContractTransmitter: provider.MedianContract(),
DataSource: dataSource,
Expand Down
75 changes: 74 additions & 1 deletion core/services/ocr2/plugins/median/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ import (
"encoding/json"
"errors"
"fmt"
"math/big"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/smartcontractkit/libocr/offchainreporting2/reportingplugin/median"
libocr "github.com/smartcontractkit/libocr/offchainreporting2plus"
ocr2types "github.com/smartcontractkit/libocr/offchainreporting2plus/types"

"github.com/smartcontractkit/chainlink-relay/pkg/loop"
"github.com/smartcontractkit/chainlink-relay/pkg/types"
Expand All @@ -22,6 +26,8 @@ import (
"github.com/smartcontractkit/chainlink/v2/plugins"
)

const ContractName = "offchain_aggregator"

type MedianConfig interface {
JobPipelineMaxSuccessfulRuns() uint64
plugins.RegistrarConfig
Expand All @@ -44,6 +50,21 @@ func (m *medianConfig) JobPipelineMaxSuccessfulRuns() uint64 {
return m.jobPipelineMaxSuccessfulRuns
}

// This wrapper avoids the need to modify the signature of NewMedianFactory in all of the non-evm
// relay repos as well as its primary definition in chainlink-relay. Once ChainReader is implemented
// and working on all 4 blockchain families, we can remove the original MedianContract() method from
// MedianProvider and pass medianContract as a separate param to NewMedianFactory
type medianProviderWrapper struct {
types.MedianProvider
contract median.MedianContract
}

// Override relay's implementation of MedianContract with product plugin's implementation of
// MedianContract, making use of product-agnostic ChainReader to read the contract instead of relay MedianContract
func (m medianProviderWrapper) MedianContract() median.MedianContract {
return m.contract
}

func NewMedianServices(ctx context.Context,
jb job.Job,
isNewlyCreatedJob bool,
Expand Down Expand Up @@ -73,7 +94,7 @@ func NewMedianServices(ctx context.Context,
JobID: jb.ID,
ContractID: spec.ContractID,
New: isNewlyCreatedJob,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
ProviderType: string(spec.PluginType),
}, types.PluginArgs{
TransmitterID: spec.TransmitterID.String,
Expand Down Expand Up @@ -111,6 +132,13 @@ func NewMedianServices(ctx context.Context,
CreatedAt: time.Now(),
}, lggr)

if medianProvider.ChainReader() != nil {
medianProvider = medianProviderWrapper{
Copy link
Contributor

Choose a reason for hiding this comment

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

@ilija42 Looking at the details of this now, I think this modification is happening too high up the call-stack. Have you considered adding a shim around median.NumericalMedianFactory that overrides the median contract with a ChainReader based implementation? This would behave almost like what @reductionista described as the ideal suggestion, which would be to modify median directly.

I think implementing it that way has a couple of advantages:

  • the shim provides a centralized, single abstraction point where we can modify median in a way that will affect all users of it.
  • easier migration -- IIRC we are going to take ownership over median in the near future, so this will make those changes easier to make.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cedric-cordenier sry, completely missed this. This makes sense to me, I will change this in the final POC version

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I also missed this somehow till today, despite being tagged!

I don't see a problem with this, but curious to understand the motivation better.

the shim provides a centralized, single abstraction point where we can modify median in a way that will affect all users of it.

What other users of NumericalMedianFactory would there be aside from the Median plugin? Is the issue just that for migration you need a separate implementation of Median plugin to co-exist with the current one? Or is it more than that?

medianProvider, // attach newer MedianContract which uses ChainReader
newMedianContract(provider.ChainReader(), common.HexToAddress(spec.ContractID)),
}
}

if cmdName := env.MedianPluginCmd.Get(); cmdName != "" {

// use unique logger names so we can use it to register a loop
Expand Down Expand Up @@ -152,3 +180,48 @@ func NewMedianServices(ctx context.Context,
}
return
}

type medianContract struct {
chainReader types.ChainReader
contract types.BoundContract
}

type latestTransmissionDetailsResponse struct {
ConfigDigest ocr2types.ConfigDigest
Epoch uint32
Round uint8
LatestAnswer *big.Int
LatestTimestamp uint64
ilija42 marked this conversation as resolved.
Show resolved Hide resolved
}

type latestRoundRequested struct {
ConfigDigest ocr2types.ConfigDigest
Epoch uint32
Round uint8
}

func (m *medianContract) LatestTransmissionDetails(ctx context.Context) (configDigest ocr2types.ConfigDigest, epoch uint32, round uint8, latestAnswer *big.Int, latestTimestamp time.Time, err error) {
var resp latestTransmissionDetailsResponse

err = m.chainReader.GetLatestValue(ctx, m.contract, "LatestTransmissionDetails", nil, &resp)
if err != nil {
return
}
return resp.ConfigDigest, resp.Epoch, resp.Round, resp.LatestAnswer, time.Unix(int64(resp.LatestTimestamp), 0), err
}

func (m *medianContract) LatestRoundRequested(ctx context.Context, lookback time.Duration) (configDigest ocr2types.ConfigDigest, epoch uint32, round uint8, err error) {
var resp latestRoundRequested

err = m.chainReader.GetLatestValue(ctx, m.contract, "LatestRoundReported", map[string]string{}, &resp)
if err != nil {
return
}

return resp.ConfigDigest, resp.Epoch, resp.Round, err
}

func newMedianContract(chainReader types.ChainReader, address common.Address) *medianContract {
contract := types.BoundContract{Address: address.String(), Name: ContractName, Pending: true}
return &medianContract{chainReader, contract}
}
2 changes: 1 addition & 1 deletion core/services/ocr2/plugins/ocr2keeper/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func EVMProvider(db *sqlx.DB, chain evm.Chain, lggr logger.Logger, spec job.Job,
ExternalJobID: spec.ExternalJobID,
JobID: oSpec.ID,
ContractID: oSpec.ContractID,
RelayConfig: oSpec.RelayConfig.Bytes(),
RelayConfig: oSpec.RelayConfig.BytesWithPreservedJson(),
},
types.PluginArgs{
TransmitterID: oSpec.TransmitterID.String,
Expand Down
8 changes: 4 additions & 4 deletions core/services/ocrbootstrap/delegate.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (d *Delegate) ServicesForSpec(jb job.Job) (services []job.ServiceCtx, err e
ctx := ctxVals.ContextWithValues(context.Background())

var routerFields relayConfigRouterContractFields
if err = json.Unmarshal(spec.RelayConfig.Bytes(), &routerFields); err != nil {
if err = json.Unmarshal(spec.RelayConfig.BytesWithPreservedJson(), &routerFields); err != nil {
return nil, err
}

Expand All @@ -124,12 +124,12 @@ func (d *Delegate) ServicesForSpec(jb job.Job) (services []job.ServiceCtx, err e
ExternalJobID: jb.ExternalJobID,
JobID: jb.ID,
ContractID: spec.ContractID,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
New: d.isNewlyCreatedJob,
ProviderType: string(types.Functions),
},
types.PluginArgs{
PluginConfig: spec.RelayConfig.Bytes(), // contains all necessary fields for config provider
PluginConfig: spec.RelayConfig.BytesWithPreservedJson(), // contains all necessary fields for config provider
},
)
} else {
Expand All @@ -138,7 +138,7 @@ func (d *Delegate) ServicesForSpec(jb job.Job) (services []job.ServiceCtx, err e
JobID: jb.ID,
ContractID: spec.ContractID,
New: d.isNewlyCreatedJob,
RelayConfig: spec.RelayConfig.Bytes(),
RelayConfig: spec.RelayConfig.BytesWithPreservedJson(),
})
}

Expand Down
Loading
Loading