From 55aeea33b26ac8b669799a3f943c116ca6bdb36b Mon Sep 17 00:00:00 2001 From: ilija Date: Sat, 15 Feb 2025 14:20:39 +0100 Subject: [PATCH 1/8] Change CR handling for Addresses not found --- .../relayinterface/chain_components_test.go | 50 +++++++++++++++++++ pkg/solana/chainreader/batch.go | 26 ++++++++-- pkg/solana/chainreader/chain_reader.go | 8 +-- 3 files changed, 75 insertions(+), 9 deletions(-) diff --git a/integration-tests/relayinterface/chain_components_test.go b/integration-tests/relayinterface/chain_components_test.go index e4d1c9ef6..2c8c72d26 100644 --- a/integration-tests/relayinterface/chain_components_test.go +++ b/integration-tests/relayinterface/chain_components_test.go @@ -261,6 +261,7 @@ func RunChainWriterTests[T WrappedTestingT[T]](t T, it *SolanaChainComponentsInt // GetLatestValue method const ( + ContractReaderNotFoundReadsReturnZeroedResponses = "Get latest value not found reads return zeroed responses" ContractReaderGetLatestValueUsingMultiReader = "Get latest value using multi reader" ContractReaderBatchGetLatestValueUsingMultiReader = "Batch Get latest value using multi reader" ContractReaderGetLatestValueWithAddressHardcodedIntoResponse = "Get latest value with AddressHardcoded into response" @@ -277,6 +278,47 @@ type TimestampedUnixBig struct { func RunContractReaderInLoopTests[T WrappedTestingT[T]](t T, it ChainComponentsInterfaceTester[T]) { //RunContractReaderInterfaceTests(t, it, false, true) testCases := []Testcase[T]{ + { + Name: ContractReaderNotFoundReadsReturnZeroedResponses, + Test: func(t T) { + cr := it.GetContractReader(t) + bindings := it.GetBindings(t) + ctx := tests.Context(t) + + bound := BindingsByName(bindings, AnyContractName)[0] + require.NoError(t, cr.Bind(ctx, bindings)) + + dAccRes := contractprimary.DataAccount{} + require.NoError(t, cr.GetLatestValue(ctx, bound.ReadIdentifier(ReadUninitializedPDA), primitives.Unconfirmed, nil, &dAccRes)) + require.Equal(t, contractprimary.DataAccount{}, dAccRes) + + mR3Res := contractprimary.MultiRead3{} + batchGetLatestValueRequest := make(types.BatchGetLatestValuesRequest) + batchGetLatestValueRequest[bound] = []types.BatchRead{ + { + ReadName: ReadUninitializedPDA, + Params: nil, + ReturnVal: &dAccRes, + }, + { + ReadName: MultiReadWithParamsReuse, + Params: map[string]any{"ID": 999}, + ReturnVal: &mR3Res, + }, + } + + batchResult, err := cr.BatchGetLatestValues(ctx, batchGetLatestValueRequest) + require.NoError(t, err) + + result, err := batchResult[bound][0].GetResult() + require.NoError(t, err) + require.Equal(t, &contractprimary.DataAccount{}, result) + + result, err = batchResult[bound][1].GetResult() + require.NoError(t, err) + require.Equal(t, &contractprimary.MultiRead3{}, result) + }, + }, { Name: ContractReaderGetLatestValueWithAddressHardcodedIntoResponse, Test: func(t T) { @@ -767,6 +809,7 @@ func (h *helper) runInitialize( } const ( + ReadUninitializedPDA = "ReadUninitializedPDA" MultiRead = "MultiRead" ReadWithAddressHardCodedIntoResponse = "ReadWithAddressHardCodedIntoResponse" MultiReadWithParamsReuse = "MultiReadWithParamsReuse" @@ -848,6 +891,13 @@ func (it *SolanaChainComponentsInterfaceTester[T]) buildContractReaderConfig(t T AnyContractName: { IDL: idl, Reads: map[string]config.ReadDefinition{ + ReadUninitializedPDA: { + ChainSpecificName: "DataAccount", + ReadType: config.Account, + PDADefinition: codec.PDATypeDef{ + Prefix: []byte("AAAAAAAAAA"), + }, + }, ReadWithAddressHardCodedIntoResponse: readWithAddressHardCodedIntoResponseDef, GetTokenPrices: { ChainSpecificName: "USDPerToken", diff --git a/pkg/solana/chainreader/batch.go b/pkg/solana/chainreader/batch.go index 35cf8d12b..6666f333e 100644 --- a/pkg/solana/chainreader/batch.go +++ b/pkg/solana/chainreader/batch.go @@ -7,7 +7,9 @@ import ( "strings" "github.com/gagliardetto/solana-go" + "github.com/gagliardetto/solana-go/rpc" + "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/types" "github.com/smartcontractkit/chainlink-common/pkg/values" ) @@ -33,7 +35,7 @@ type MultipleAccountGetter interface { } // doMultiRead aggregate results from multiple PDAs from the same contract into one result. -func doMultiRead(ctx context.Context, client MultipleAccountGetter, bdRegistry *bindingsRegistry, rv readValues, params, returnValue any) error { +func doMultiRead(ctx context.Context, lggr logger.Logger, client MultipleAccountGetter, bdRegistry *bindingsRegistry, rv readValues, params, returnValue any) error { batch := make([]call, len(rv.reads)) for idx, r := range rv.reads { batch[idx] = call{ @@ -46,7 +48,7 @@ func doMultiRead(ctx context.Context, client MultipleAccountGetter, bdRegistry * } } - results, err := doMethodBatchCall(ctx, client, bdRegistry, batch) + results, err := doMethodBatchCall(ctx, lggr, client, bdRegistry, batch) if err != nil { return err } @@ -69,7 +71,7 @@ func doMultiRead(ctx context.Context, client MultipleAccountGetter, bdRegistry * return nil } -func doMethodBatchCall(ctx context.Context, client MultipleAccountGetter, bdRegistry *bindingsRegistry, batch []call) ([]batchResultWithErr, error) { +func doMethodBatchCall(ctx context.Context, lggr logger.Logger, client MultipleAccountGetter, bdRegistry *bindingsRegistry, batch []call) ([]batchResultWithErr, error) { // Create the list of public keys to fetch keys := make([]solana.PublicKey, len(batch)) for idx, batchCall := range batch { @@ -85,13 +87,27 @@ func doMethodBatchCall(ctx context.Context, client MultipleAccountGetter, bdRegi } // Fetch the account data + results := make([]batchResultWithErr, len(batch)) data, err := client.GetMultipleAccountData(ctx, keys...) if err != nil { + if errors.Is(err, rpc.ErrNotFound) { + var sb strings.Builder + sb.WriteString(fmt.Sprintf("failed to get multiple account data with err: %s, now returning empty responses for: \n", err)) + for i, c := range batch { + sb.WriteString(fmt.Sprintf("- call: #%d under namespace %s, with readName: %q and address: %q\n", i+1, c.Namespace, c.ReadName, keys[i])) + results[i] = batchResultWithErr{ + address: keys[i].String(), + namespace: c.Namespace, + readName: c.ReadName, + returnVal: c.ReturnVal, + } + } + lggr.Infof(sb.String()) + return results, nil + } return nil, err } - results := make([]batchResultWithErr, len(batch)) - // decode batch call results for idx, batchCall := range batch { results[idx] = batchResultWithErr{ diff --git a/pkg/solana/chainreader/chain_reader.go b/pkg/solana/chainreader/chain_reader.go index 0ee1f5a61..419b2ac40 100644 --- a/pkg/solana/chainreader/chain_reader.go +++ b/pkg/solana/chainreader/chain_reader.go @@ -183,7 +183,7 @@ func (s *ContractReaderService) GetLatestValue(ctx context.Context, readIdentifi } if len(values.reads) > 1 { - return doMultiRead(ctx, s.client, s.bdRegistry, values, params, returnVal) + return doMultiRead(ctx, s.lggr, s.client, s.bdRegistry, values, params, returnVal) } // TODO this is a temporary edge case - NONEVM-1320 @@ -203,7 +203,7 @@ func (s *ContractReaderService) GetLatestValue(ctx context.Context, readIdentifi }, } - results, err := doMethodBatchCall(ctx, s.client, s.bdRegistry, batch) + results, err := doMethodBatchCall(ctx, s.lggr, s.client, s.bdRegistry, batch) if err != nil { return err } @@ -244,7 +244,7 @@ func (s *ContractReaderService) BatchGetLatestValues(ctx context.Context, reques // exclude multi read reads from the big batch request and populate them separately and merge results later. if len(vals.reads) > 1 { - err := doMultiRead(ctx, s.client, s.bdRegistry, vals, readReq.Params, readReq.ReturnVal) + err := doMultiRead(ctx, s.lggr, s.client, s.bdRegistry, vals, readReq.Params, readReq.ReturnVal) multiIdxLookup[bound][idx] = len(multiReadResults) multiReadResults = append(multiReadResults, batchResultWithErr{address: vals.address, namespace: vals.contract, readName: readReq.ReadName, returnVal: readReq.ReturnVal, err: err}) @@ -268,7 +268,7 @@ func (s *ContractReaderService) BatchGetLatestValues(ctx context.Context, reques } } - results, err := doMethodBatchCall(ctx, s.client, s.bdRegistry, batch) + results, err := doMethodBatchCall(ctx, s.lggr, s.client, s.bdRegistry, batch) if err != nil { return nil, err } From cba7b51d385b65d88e72a203ce57c4c8f9949271 Mon Sep 17 00:00:00 2001 From: ilija Date: Sat, 15 Feb 2025 14:23:49 +0100 Subject: [PATCH 2/8] lint --- integration-tests/relayinterface/chain_components_test.go | 5 ----- pkg/solana/chainreader/batch.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/integration-tests/relayinterface/chain_components_test.go b/integration-tests/relayinterface/chain_components_test.go index 2c8c72d26..5cc6b07a1 100644 --- a/integration-tests/relayinterface/chain_components_test.go +++ b/integration-tests/relayinterface/chain_components_test.go @@ -270,11 +270,6 @@ const ( ChainWriterLookupTableTest = "Set contract value using a lookup table for addresses" ) -type TimestampedUnixBig struct { - Value *big.Int `json:"value"` - Timestamp uint32 `json:"timestamp"` -} - func RunContractReaderInLoopTests[T WrappedTestingT[T]](t T, it ChainComponentsInterfaceTester[T]) { //RunContractReaderInterfaceTests(t, it, false, true) testCases := []Testcase[T]{ diff --git a/pkg/solana/chainreader/batch.go b/pkg/solana/chainreader/batch.go index 6666f333e..4836654b7 100644 --- a/pkg/solana/chainreader/batch.go +++ b/pkg/solana/chainreader/batch.go @@ -102,7 +102,7 @@ func doMethodBatchCall(ctx context.Context, lggr logger.Logger, client MultipleA returnVal: c.ReturnVal, } } - lggr.Infof(sb.String()) + lggr.Info(sb.String()) return results, nil } return nil, err From e409541ef746a6b809f7d532549710b0f9e5565a Mon Sep 17 00:00:00 2001 From: ilija42 <57732589+ilija42@users.noreply.github.com> Date: Mon, 17 Feb 2025 01:10:15 +0100 Subject: [PATCH 3/8] Improve account reading err handling and fix get token prices for a non loop call (#1079) * Handle get token prices for map params * Fix getPDAsForGetTokenPrices handling for non Loop call * Fix getPDAsForGetTokenPrices handling for non Loop call * Tidy * lint --- go.mod | 2 +- .../relayinterface/chain_components_test.go | 173 +++++++++++------- pkg/solana/chainreader/chain_reader.go | 72 +++++++- 3 files changed, 165 insertions(+), 82 deletions(-) diff --git a/go.mod b/go.mod index 539941f03..56dfcb2b5 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/hashicorp/go-plugin v1.6.2 github.com/jackc/pgx/v4 v4.18.3 github.com/lib/pq v1.10.9 + github.com/mitchellh/mapstructure v1.5.0 github.com/pelletier/go-toml/v2 v2.2.3 github.com/prometheus/client_golang v1.20.5 github.com/smartcontractkit/chainlink-ccip v0.0.0-20250203132120-f0d42463e405 @@ -99,7 +100,6 @@ require ( github.com/mattn/go-sqlite3 v2.0.3+incompatible // indirect github.com/miekg/dns v1.1.61 // indirect github.com/mitchellh/go-testing-interface v1.14.1 // indirect - github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/moby/sys/sequential v0.6.0 // indirect github.com/moby/sys/user v0.3.0 // indirect github.com/moby/sys/userns v0.1.0 // indirect diff --git a/integration-tests/relayinterface/chain_components_test.go b/integration-tests/relayinterface/chain_components_test.go index 5cc6b07a1..12c1a96b6 100644 --- a/integration-tests/relayinterface/chain_components_test.go +++ b/integration-tests/relayinterface/chain_components_test.go @@ -113,79 +113,113 @@ func DisableTests(it *SolanaChainComponentsInterfaceTester[*testing.T]) { } func RunChainComponentsSolanaTests[T WrappedTestingT[T]](t T, it *SolanaChainComponentsInterfaceTester[T]) { - testCases := Testcase[T]{ - Name: "Test address groups where first namespace shares address with second namespace", - Test: func(t T) { - ctx := tests.Context(t) - cfg := it.buildContractReaderConfig(t) - cfg.AddressShareGroups = [][]string{{AnyContractNameWithSharedAddress1, AnyContractNameWithSharedAddress2, AnyContractNameWithSharedAddress3}} - cr := it.GetContractReaderWithCustomCfg(t, cfg) - - t.Run("Namespace is part of an address share group that doesn't have a registered address and provides no address during Bind", func(t T) { - bound1 := []types.BoundContract{{ - Name: AnyContractNameWithSharedAddress1, - }} - require.Error(t, cr.Bind(ctx, bound1)) - }) - - addressToBeShared := it.Helper.CreateAccount(t, *it, AnyContractName, AnyValueToReadWithoutAnArgument, CreateTestStruct(0, it)).String() - t.Run("Namespace is part of an address share group that doesn't have a registered address and provides an address during Bind", func(t T) { - bound1 := []types.BoundContract{{Name: AnyContractNameWithSharedAddress1, Address: addressToBeShared}} - - require.NoError(t, cr.Bind(ctx, bound1)) + testCases := []Testcase[T]{ + { + Name: "Test address groups where first namespace shares address with second namespace", + Test: func(t T) { + ctx := tests.Context(t) + cfg := it.buildContractReaderConfig(t) + cfg.AddressShareGroups = [][]string{{AnyContractNameWithSharedAddress1, AnyContractNameWithSharedAddress2, AnyContractNameWithSharedAddress3}} + cr := it.GetContractReaderWithCustomCfg(t, cfg) + + t.Run("Namespace is part of an address share group that doesn't have a registered address and provides no address during Bind", func(t T) { + bound1 := []types.BoundContract{{ + Name: AnyContractNameWithSharedAddress1, + }} + require.Error(t, cr.Bind(ctx, bound1)) + }) + + addressToBeShared := it.Helper.CreateAccount(t, *it, AnyContractName, AnyValueToReadWithoutAnArgument, CreateTestStruct(0, it)).String() + t.Run("Namespace is part of an address share group that doesn't have a registered address and provides an address during Bind", func(t T) { + bound1 := []types.BoundContract{{Name: AnyContractNameWithSharedAddress1, Address: addressToBeShared}} + + require.NoError(t, cr.Bind(ctx, bound1)) + + var prim uint64 + require.NoError(t, cr.GetLatestValue(ctx, bound1[0].ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) + assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) + }) + + t.Run("Namespace is part of an address share group that has a registered address and provides that same address during Bind", func(t T) { + bound2 := []types.BoundContract{{ + Name: AnyContractNameWithSharedAddress2, + Address: addressToBeShared}} + require.NoError(t, cr.Bind(ctx, bound2)) + + var prim uint64 + require.NoError(t, cr.GetLatestValue(ctx, bound2[0].ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) + assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) + assert.Equal(t, addressToBeShared, bound2[0].Address) + }) + + t.Run("Namespace is part of an address share group that has a registered address and provides a wrong address during Bind", func(t T) { + key, err := solana.NewRandomPrivateKey() + require.NoError(t, err) + + bound2 := []types.BoundContract{{ + Name: AnyContractNameWithSharedAddress2, + Address: key.PublicKey().String()}} + require.Error(t, cr.Bind(ctx, bound2)) + }) + + t.Run("Namespace is part of an address share group that has a registered address and provides no address during Bind", func(t T) { + bound3 := []types.BoundContract{{Name: AnyContractNameWithSharedAddress3}} + require.NoError(t, cr.Bind(ctx, bound3)) + + var prim uint64 + require.NoError(t, cr.GetLatestValue(ctx, bound3[0].ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) + assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) + assert.Equal(t, addressToBeShared, bound3[0].Address) + + // when run in a loop Bind address won't be set, so check if CR Method works without set address. + prim = 0 + require.NoError(t, cr.GetLatestValue(ctx, types.BoundContract{ + Address: "", + Name: AnyContractNameWithSharedAddress3, + }.ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) + assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) + }) + + t.Run("Namespace is not part of an address share group that has a registered address and provides no address during Bind", func(t T) { + require.Error(t, cr.Bind(ctx, []types.BoundContract{{Name: AnyContractName}})) + }) + }, + }, - var prim uint64 - require.NoError(t, cr.GetLatestValue(ctx, bound1[0].ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) - assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) - }) + {Name: ContractReaderGetLatestValueGetTokenPrices, + Test: func(t T) { + cr := it.GetContractReader(t) + bindings := it.GetBindings(t) + ctx := tests.Context(t) - t.Run("Namespace is part of an address share group that has a registered address and provides that same address during Bind", func(t T) { - bound2 := []types.BoundContract{{ - Name: AnyContractNameWithSharedAddress2, - Address: addressToBeShared}} - require.NoError(t, cr.Bind(ctx, bound2)) + bound := BindingsByName(bindings, AnyContractName)[0] - var prim uint64 - require.NoError(t, cr.GetLatestValue(ctx, bound2[0].ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) - assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) - assert.Equal(t, addressToBeShared, bound2[0].Address) - }) + require.NoError(t, cr.Bind(ctx, bindings)) - t.Run("Namespace is part of an address share group that has a registered address and provides a wrong address during Bind", func(t T) { - key, err := solana.NewRandomPrivateKey() - require.NoError(t, err) + type TimestampedUnixBig struct { + Value *big.Int `json:"value"` + Timestamp uint32 `json:"timestamp"` + } - bound2 := []types.BoundContract{{ - Name: AnyContractNameWithSharedAddress2, - Address: key.PublicKey().String()}} - require.Error(t, cr.Bind(ctx, bound2)) - }) + res := make([]TimestampedUnixBig, 2) - t.Run("Namespace is part of an address share group that has a registered address and provides no address during Bind", func(t T) { - bound3 := []types.BoundContract{{Name: AnyContractNameWithSharedAddress3}} - require.NoError(t, cr.Bind(ctx, bound3)) + byteTokens := make([][]byte, 0, 2) + pubKey1, err := solana.PublicKeyFromBase58(GetTokenPricesPubKey1) + require.NoError(t, err) + pubKey2, err := solana.PublicKeyFromBase58(GetTokenPricesPubKey2) + require.NoError(t, err) - var prim uint64 - require.NoError(t, cr.GetLatestValue(ctx, bound3[0].ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) - assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) - assert.Equal(t, addressToBeShared, bound3[0].Address) - - // when run in a loop Bind address won't be set, so check if CR Method works without set address. - prim = 0 - require.NoError(t, cr.GetLatestValue(ctx, types.BoundContract{ - Address: "", - Name: AnyContractNameWithSharedAddress3, - }.ReadIdentifier(MethodReturningUint64), primitives.Unconfirmed, nil, &prim)) - assert.Equal(t, AnyValueToReadWithoutAnArgument, prim) - }) - - t.Run("Namespace is not part of an address share group that has a registered address and provides no address during Bind", func(t T) { - require.Error(t, cr.Bind(ctx, []types.BoundContract{{Name: AnyContractName}})) - }) - }, + byteTokens = append(byteTokens, pubKey1.Bytes()) + byteTokens = append(byteTokens, pubKey2.Bytes()) + require.NoError(t, cr.GetLatestValue(ctx, bound.ReadIdentifier(GetTokenPrices), primitives.Unconfirmed, map[string]any{"tokens": byteTokens}, &res)) + require.Equal(t, "7048352069843304521481572571769838000081483315549204879493368331", res[0].Value.String()) + require.Equal(t, uint32(1700000001), res[0].Timestamp) + require.Equal(t, "17980346130170174053328187512531209543631592085982266692926093439168", res[1].Value.String()) + require.Equal(t, uint32(1800000002), res[1].Timestamp) + }}, } - RunTests(t, it, []Testcase[T]{testCases}) + RunTests(t, it, testCases) RunContractReaderTests(t, it) RunChainWriterTests(t, it) } @@ -334,8 +368,9 @@ func RunContractReaderInLoopTests[T WrappedTestingT[T]](t T, it ChainComponentsI AddressToShare []byte } + c := int16(0) mRR := MultiReadResult{} - require.NoError(t, cr.GetLatestValue(ctx, bound.ReadIdentifier(ReadWithAddressHardCodedIntoResponse), primitives.Unconfirmed, nil, &mRR)) + require.NoError(t, cr.GetLatestValue(ctx, bound.ReadIdentifier(ReadWithAddressHardCodedIntoResponse), primitives.Unconfirmed, nil, &c)) expectedMRR := MultiReadResult{A: 1, B: 2, SharedAddress: boundAddress.Bytes(), AddressToShare: boundAddress.Bytes()} require.Equal(t, expectedMRR, mRR) @@ -841,12 +876,8 @@ func (it *SolanaChainComponentsInterfaceTester[T]) buildContractReaderConfig(t T PDADefinition: codec.PDATypeDef{ Prefix: []byte("multi_read1"), }, - ResponseAddressHardCoder: &commoncodec.HardCodeModifierConfig{ - // placeholder values, whatever is put as value gets replaced with a solana pub key anyway - OffChainValues: map[string]any{ - "SharedAddress": "", - "AddressToShare": "", - }, + OutputModifications: commoncodec.ModifiersConfig{ + &commoncodec.PropertyExtractorConfig{FieldName: "B"}, }, } diff --git a/pkg/solana/chainreader/chain_reader.go b/pkg/solana/chainreader/chain_reader.go index 419b2ac40..7bfff527a 100644 --- a/pkg/solana/chainreader/chain_reader.go +++ b/pkg/solana/chainreader/chain_reader.go @@ -13,6 +13,7 @@ import ( bin "github.com/gagliardetto/binary" "github.com/gagliardetto/solana-go" "github.com/gagliardetto/solana-go/rpc" + "github.com/mitchellh/mapstructure" "github.com/smartcontractkit/chainlink-ccip/chains/solana/gobindings/fee_quoter" commoncodec "github.com/smartcontractkit/chainlink-common/pkg/codec" @@ -696,6 +697,11 @@ func (s *ContractReaderService) handleGetTokenPricesGetLatestValue( return err } + if len(pdaAddresses) == 0 { + s.lggr.Infof("No token addresses found in params: %v that were passed into %q, call to contract: %q with address: %q", params, GetTokenPrices, values.contract, values.address) + return nil + } + data, err := s.client.GetMultipleAccountData(ctx, pdaAddresses...) if err != nil { return err @@ -705,7 +711,32 @@ func (s *ContractReaderService) handleGetTokenPricesGetLatestValue( if returnSliceVal.Kind() != reflect.Ptr { return fmt.Errorf("expected <**[]*struct { Value *big.Int; Timestamp *int64 } Value>, got %q", returnSliceVal.String()) } + returnSliceVal = returnSliceVal.Elem() + // if called directly instead of as a loop + if returnSliceVal.Kind() == reflect.Slice { + underlyingType := returnSliceVal.Type().Elem() + if underlyingType.Kind() == reflect.Struct { + if _, hasValue := underlyingType.FieldByName("Value"); hasValue { + if _, hasTimestamp := underlyingType.FieldByName("Timestamp"); hasTimestamp { + sliceVal := reflect.MakeSlice(returnSliceVal.Type(), 0, 0) + for _, d := range data { + var wrapper fee_quoter.BillingTokenConfigWrapper + if err = wrapper.UnmarshalWithDecoder(bin.NewBorshDecoder(d)); err != nil { + return err + } + newElem := reflect.New(underlyingType).Elem() + newElem.FieldByName("Value").Set(reflect.ValueOf(big.NewInt(0).SetBytes(wrapper.Config.UsdPerToken.Value[:]))) + // nolint:gosec + // G115: integer overflow conversion int64 -> uint32 + newElem.FieldByName("Timestamp").Set(reflect.ValueOf(uint32(wrapper.Config.UsdPerToken.Timestamp))) + sliceVal = reflect.Append(sliceVal, newElem) + } + return mapstructure.Decode(sliceVal.Interface(), returnVal) + } + } + } + } returnSliceValType := returnSliceVal.Type() if returnSliceValType.Kind() != reflect.Ptr { @@ -771,25 +802,46 @@ func (s *ContractReaderService) getPDAsForGetTokenPrices(params any, values read if val.Kind() == reflect.Ptr { val = val.Elem() } - if val.Kind() != reflect.Struct { + + var field reflect.Value + switch val.Kind() { + case reflect.Struct: + field = val.FieldByName("Tokens") + if !field.IsValid() { + field = val.FieldByName("tokens") + } + case reflect.Map: + field = val.MapIndex(reflect.ValueOf("Tokens")) + if !field.IsValid() { + field = val.MapIndex(reflect.ValueOf("tokens")) + } + default: return nil, fmt.Errorf( - "for contract %q read %q: expected `params` to be a struct, got %s", - values.contract, values.reads[0].readName, val.Kind(), + "for contract %q read %q: expected `params` to be a struct or map, got %q: %q", + values.contract, values.reads[0].readName, val.Kind(), val.String(), ) } - field := val.FieldByName("Tokens") if !field.IsValid() { return nil, fmt.Errorf( - "for contract %q read %q: no field named 'Tokens' found in params", - values.contract, values.reads[0].readName, + "for contract %q read %q: no field named 'Tokens' found in kind: %q: %q", + values.contract, values.reads[0].readName, val.Kind(), val.String(), ) } - tokens, ok := field.Interface().(*[][32]uint8) - if !ok { + var tokens [][]uint8 + switch x := field.Interface().(type) { + // this is the type when CR is called as LOOP and creates types from IDL + case *[][32]uint8: + for _, arr := range *x { + tokens = append(tokens, arr[:]) // Slice [32]uint8 → []uint8 + } + // this is the expected type when CR is called directly + case [][]uint8: + tokens = x + default: return nil, fmt.Errorf( - "for contract %q read %q: 'Tokens' field is not of type *[][32]uint8", + "for contract %q read %q: 'Tokens' field is neither *[][32]uint8 nor [][]uint8", values.contract, values.reads[0].readName, ) } @@ -804,7 +856,7 @@ func (s *ContractReaderService) getPDAsForGetTokenPrices(params any, values read // Build the PDA addresses for all tokens. var pdaAddresses []solana.PublicKey - for _, token := range *tokens { + for _, token := range tokens { tokenAddr := solana.PublicKeyFromBytes(token[:]) if !tokenAddr.IsOnCurve() || tokenAddr.IsZero() { return nil, fmt.Errorf( From 271a6408262a7945ea1e745a8fea329e31fffe40 Mon Sep 17 00:00:00 2001 From: ilija Date: Tue, 18 Feb 2025 01:05:23 +0100 Subject: [PATCH 4/8] fix cr test --- integration-tests/relayinterface/chain_components_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/integration-tests/relayinterface/chain_components_test.go b/integration-tests/relayinterface/chain_components_test.go index 12c1a96b6..6c04ae59a 100644 --- a/integration-tests/relayinterface/chain_components_test.go +++ b/integration-tests/relayinterface/chain_components_test.go @@ -876,8 +876,12 @@ func (it *SolanaChainComponentsInterfaceTester[T]) buildContractReaderConfig(t T PDADefinition: codec.PDATypeDef{ Prefix: []byte("multi_read1"), }, - OutputModifications: commoncodec.ModifiersConfig{ - &commoncodec.PropertyExtractorConfig{FieldName: "B"}, + ResponseAddressHardCoder: &commoncodec.HardCodeModifierConfig{ + // placeholder values, whatever is put as value gets replaced with a solana pub key anyway + OffChainValues: map[string]any{ + "SharedAddress": "", + "AddressToShare": "", + }, }, } From 47be6c25af1267767a5082a9a4a39ffa99d154b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Tue, 18 Feb 2025 17:35:24 +0900 Subject: [PATCH 5/8] Refactor fix, don't discard account data from accounts we were able to read --- pkg/solana/chainreader/batch.go | 26 +++++------------------- pkg/solana/chainreader/chain_reader.go | 14 +++++++++---- pkg/solana/chainreader/client_wrapper.go | 14 +++---------- 3 files changed, 18 insertions(+), 36 deletions(-) diff --git a/pkg/solana/chainreader/batch.go b/pkg/solana/chainreader/batch.go index 4836654b7..0749efd81 100644 --- a/pkg/solana/chainreader/batch.go +++ b/pkg/solana/chainreader/batch.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/gagliardetto/solana-go" - "github.com/gagliardetto/solana-go/rpc" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/types" @@ -90,43 +89,28 @@ func doMethodBatchCall(ctx context.Context, lggr logger.Logger, client MultipleA results := make([]batchResultWithErr, len(batch)) data, err := client.GetMultipleAccountData(ctx, keys...) if err != nil { - if errors.Is(err, rpc.ErrNotFound) { - var sb strings.Builder - sb.WriteString(fmt.Sprintf("failed to get multiple account data with err: %s, now returning empty responses for: \n", err)) - for i, c := range batch { - sb.WriteString(fmt.Sprintf("- call: #%d under namespace %s, with readName: %q and address: %q\n", i+1, c.Namespace, c.ReadName, keys[i])) - results[i] = batchResultWithErr{ - address: keys[i].String(), - namespace: c.Namespace, - readName: c.ReadName, - returnVal: c.ReturnVal, - } - } - lggr.Info(sb.String()) - return results, nil - } return nil, err } // decode batch call results for idx, batchCall := range batch { + address := keys[idx] results[idx] = batchResultWithErr{ - address: keys[idx].String(), + address: address.String(), namespace: batchCall.Namespace, readName: batchCall.ReadName, returnVal: batchCall.ReturnVal, } - if data[idx] == nil || len(data[idx]) == 0 { - results[idx].err = ErrMissingAccountData - + if len(data[idx]) == 0 { + lggr.Infow("failed to find account, returning zero value instead", "namespace", batchCall.Namespace, "readName", batchCall.ReadName, "address", address) + // TODO: add flag to enforce returning an error, e.g. ErrMissingAccountData continue } rBinding, err := bdRegistry.GetReader(results[idx].namespace, results[idx].readName) if err != nil { results[idx].err = err - continue } diff --git a/pkg/solana/chainreader/chain_reader.go b/pkg/solana/chainreader/chain_reader.go index 7bfff527a..ada7c99ac 100644 --- a/pkg/solana/chainreader/chain_reader.go +++ b/pkg/solana/chainreader/chain_reader.go @@ -722,8 +722,11 @@ func (s *ContractReaderService) handleGetTokenPricesGetLatestValue( sliceVal := reflect.MakeSlice(returnSliceVal.Type(), 0, 0) for _, d := range data { var wrapper fee_quoter.BillingTokenConfigWrapper - if err = wrapper.UnmarshalWithDecoder(bin.NewBorshDecoder(d)); err != nil { - return err + // if we got back an empty account then the account must not exist yet, use zero value + if len(d) > 0 { + if err = wrapper.UnmarshalWithDecoder(bin.NewBorshDecoder(d)); err != nil { + return err + } } newElem := reflect.New(underlyingType).Elem() newElem.FieldByName("Value").Set(reflect.ValueOf(big.NewInt(0).SetBytes(wrapper.Config.UsdPerToken.Value[:]))) @@ -772,8 +775,11 @@ func (s *ContractReaderService) handleGetTokenPricesGetLatestValue( for _, d := range data { var wrapper fee_quoter.BillingTokenConfigWrapper - if err = wrapper.UnmarshalWithDecoder(bin.NewBorshDecoder(d)); err != nil { - return err + // if we got back an empty account then the account must not exist yet, use zero value + if len(d) > 0 { + if err = wrapper.UnmarshalWithDecoder(bin.NewBorshDecoder(d)); err != nil { + return err + } } newElemPtr := reflect.New(underlyingStruct) diff --git a/pkg/solana/chainreader/client_wrapper.go b/pkg/solana/chainreader/client_wrapper.go index 4755d93d6..8196e1cde 100644 --- a/pkg/solana/chainreader/client_wrapper.go +++ b/pkg/solana/chainreader/client_wrapper.go @@ -28,18 +28,10 @@ func (w *RPCClientWrapper) GetMultipleAccountData(ctx context.Context, keys ...s bts := make([][]byte, len(result.Value)) for idx, res := range result.Value { - if res == nil { - return nil, rpc.ErrNotFound + if res == nil || res.Data == nil || res.Data.GetBinary() == nil { + // any accounts that can't be resolved will be nil + continue } - - if res.Data == nil { - return nil, rpc.ErrNotFound - } - - if res.Data.GetBinary() == nil { - return nil, rpc.ErrNotFound - } - bts[idx] = res.Data.GetBinary() } From b12de9e284da38576df87eedce810d5d4bca04e7 Mon Sep 17 00:00:00 2001 From: ilija Date: Tue, 18 Feb 2025 11:05:50 +0100 Subject: [PATCH 6/8] Implement ErrOnMissingAccountData flag for CR calls --- pkg/solana/chainreader/batch.go | 17 +++++++++----- pkg/solana/chainreader/chain_reader.go | 25 ++++++++++++--------- pkg/solana/chainreader/chain_reader_test.go | 8 ++++--- pkg/solana/chainreader/lookup.go | 2 +- pkg/solana/config/chain_reader.go | 15 +++++++------ 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/pkg/solana/chainreader/batch.go b/pkg/solana/chainreader/batch.go index 0749efd81..1982422e2 100644 --- a/pkg/solana/chainreader/batch.go +++ b/pkg/solana/chainreader/batch.go @@ -14,8 +14,9 @@ import ( ) type call struct { - Namespace, ReadName string - Params, ReturnVal any + Namespace, ReadName string + Params, ReturnVal any + ErrOnMissingAccountData bool } type batchResultWithErr struct { @@ -38,9 +39,10 @@ func doMultiRead(ctx context.Context, lggr logger.Logger, client MultipleAccount batch := make([]call, len(rv.reads)) for idx, r := range rv.reads { batch[idx] = call{ - Namespace: rv.contract, - ReadName: r.readName, - ReturnVal: returnValue, + Namespace: rv.contract, + ReadName: r.readName, + ReturnVal: returnValue, + ErrOnMissingAccountData: r.errOnMissingAccountData, } if r.useParams { batch[idx].Params = params @@ -103,8 +105,11 @@ func doMethodBatchCall(ctx context.Context, lggr logger.Logger, client MultipleA } if len(data[idx]) == 0 { + if batchCall.ErrOnMissingAccountData { + results[idx].err = ErrMissingAccountData + continue + } lggr.Infow("failed to find account, returning zero value instead", "namespace", batchCall.Namespace, "readName", batchCall.ReadName, "address", address) - // TODO: add flag to enforce returning an error, e.g. ErrMissingAccountData continue } diff --git a/pkg/solana/chainreader/chain_reader.go b/pkg/solana/chainreader/chain_reader.go index ada7c99ac..2848b1279 100644 --- a/pkg/solana/chainreader/chain_reader.go +++ b/pkg/solana/chainreader/chain_reader.go @@ -197,10 +197,11 @@ func (s *ContractReaderService) GetLatestValue(ctx context.Context, readIdentifi batch := []call{ { - Namespace: values.contract, - ReadName: values.reads[0].readName, - Params: params, - ReturnVal: returnVal, + Namespace: values.contract, + ReadName: values.reads[0].readName, + Params: params, + ReturnVal: returnVal, + ErrOnMissingAccountData: values.reads[0].errOnMissingAccountData, }, } @@ -261,10 +262,11 @@ func (s *ContractReaderService) BatchGetLatestValues(ctx context.Context, reques } batch = append(batch, call{ - Namespace: bound.Name, - ReadName: readReq.ReadName, - Params: readReq.Params, - ReturnVal: readReq.ReturnVal, + Namespace: bound.Name, + ReadName: readReq.ReadName, + Params: readReq.Params, + ReturnVal: readReq.ReturnVal, + ErrOnMissingAccountData: vals.reads[0].errOnMissingAccountData, }) } } @@ -446,7 +448,7 @@ func (s *ContractReaderService) initNamespace(namespaces map[string]config.Chain } func (s *ContractReaderService) addAccountRead(namespace string, genericName string, idl codec.IDL, outputIDLDef codec.IdlTypeDef, readDefinition config.ReadDefinition) error { - reads := []read{{readName: genericName, useParams: true}} + reads := []read{{readName: genericName, useParams: true, errOnMissingAccountData: readDefinition.ErrOnMissingAccountData}} if readDefinition.MultiReader != nil { multiRead, err := s.addMultiAccountReadToCodec(namespace, readDefinition, idl) if err != nil { @@ -517,8 +519,9 @@ func (s *ContractReaderService) addMultiAccountReadToCodec(namespace string, rea s.bdRegistry.AddReader(namespace, genericName, newAccountReadBinding(namespace, genericName, isPDA, mr.PDADefinition.Prefix, idl, inputIDLDef, accountIDLDef, readDefinition)) reads = append(reads, read{ - readName: genericName, - useParams: readDefinition.MultiReader.ReuseParams, + readName: genericName, + useParams: readDefinition.MultiReader.ReuseParams, + errOnMissingAccountData: mr.ErrOnMissingAccountData, }) } diff --git a/pkg/solana/chainreader/chain_reader_test.go b/pkg/solana/chainreader/chain_reader_test.go index eb905b6bf..e087047f9 100644 --- a/pkg/solana/chainreader/chain_reader_test.go +++ b/pkg/solana/chainreader/chain_reader_test.go @@ -16,11 +16,12 @@ import ( "github.com/cometbft/cometbft/libs/service" "github.com/gagliardetto/solana-go" "github.com/google/uuid" - "github.com/smartcontractkit/chainlink-common/pkg/sqlutil/sqltest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink-common/pkg/sqlutil/sqltest" + "github.com/smartcontractkit/libocr/commontypes" codeccommon "github.com/smartcontractkit/chainlink-common/pkg/codec" @@ -620,8 +621,9 @@ func newTestConfAndCodec(t *testing.T) (types.RemoteCodec, config.ContractReader IDL: mustUnmarshalIDL(t, rawIDL), Reads: map[string]config.ReadDefinition{ NamedMethod: { - ChainSpecificName: testutils.TestStructWithNestedStruct, - ReadType: config.Account, + ChainSpecificName: testutils.TestStructWithNestedStruct, + ReadType: config.Account, + ErrOnMissingAccountData: true, OutputModifications: codeccommon.ModifiersConfig{ &codeccommon.RenameModifierConfig{Fields: map[string]string{"Value": "V"}}, }, diff --git a/pkg/solana/chainreader/lookup.go b/pkg/solana/chainreader/lookup.go index ef7b1a551..1ca9c6dc4 100644 --- a/pkg/solana/chainreader/lookup.go +++ b/pkg/solana/chainreader/lookup.go @@ -9,7 +9,7 @@ import ( type read struct { readName string // useParams is used when this read is part of a multi read to determine if it should use parent read params. - useParams bool + useParams, errOnMissingAccountData bool } type readValues struct { diff --git a/pkg/solana/config/chain_reader.go b/pkg/solana/config/chain_reader.go index dfeae9e6d..7dd6921a9 100644 --- a/pkg/solana/config/chain_reader.go +++ b/pkg/solana/config/chain_reader.go @@ -83,13 +83,14 @@ type MultiReader struct { } type ReadDefinition struct { - ChainSpecificName string `json:"chainSpecificName"` - ReadType ReadType `json:"readType,omitempty"` - InputModifications commoncodec.ModifiersConfig `json:"inputModifications,omitempty"` - OutputModifications commoncodec.ModifiersConfig `json:"outputModifications,omitempty"` - PDADefinition codec.PDATypeDef `json:"pdaDefinition,omitempty"` // Only used for PDA account reads - MultiReader *MultiReader `json:"multiReader,omitempty"` - EventDefinitions *EventDefinitions `json:"eventDefinitions,omitempty"` + ChainSpecificName string `json:"chainSpecificName"` + ReadType ReadType `json:"readType,omitempty"` + ErrOnMissingAccountData bool `json:"errOnMissingAccountData,omitempty"` + InputModifications commoncodec.ModifiersConfig `json:"inputModifications,omitempty"` + OutputModifications commoncodec.ModifiersConfig `json:"outputModifications,omitempty"` + PDADefinition codec.PDATypeDef `json:"pdaDefinition,omitempty"` // Only used for PDA account reads + MultiReader *MultiReader `json:"multiReader,omitempty"` + EventDefinitions *EventDefinitions `json:"eventDefinitions,omitempty"` // ResponseAddressHardCoder hardcodes the address of the contract into the defined field in the response. ResponseAddressHardCoder *commoncodec.HardCodeModifierConfig `json:"responseAddressHardCoder,omitempty"` } From 4b52f35862b413f54a750908afe03410e4f4a9af Mon Sep 17 00:00:00 2001 From: ilija Date: Tue, 18 Feb 2025 14:39:05 +0100 Subject: [PATCH 7/8] cleanup --- integration-tests/relayinterface/chain_components_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration-tests/relayinterface/chain_components_test.go b/integration-tests/relayinterface/chain_components_test.go index 6c04ae59a..c00371db4 100644 --- a/integration-tests/relayinterface/chain_components_test.go +++ b/integration-tests/relayinterface/chain_components_test.go @@ -368,9 +368,8 @@ func RunContractReaderInLoopTests[T WrappedTestingT[T]](t T, it ChainComponentsI AddressToShare []byte } - c := int16(0) mRR := MultiReadResult{} - require.NoError(t, cr.GetLatestValue(ctx, bound.ReadIdentifier(ReadWithAddressHardCodedIntoResponse), primitives.Unconfirmed, nil, &c)) + require.NoError(t, cr.GetLatestValue(ctx, bound.ReadIdentifier(ReadWithAddressHardCodedIntoResponse), primitives.Unconfirmed, nil, &mRR)) expectedMRR := MultiReadResult{A: 1, B: 2, SharedAddress: boundAddress.Bytes(), AddressToShare: boundAddress.Bytes()} require.Equal(t, expectedMRR, mRR) From b39c52eba7b53ac3525052629f7dbff49600d33d Mon Sep 17 00:00:00 2001 From: ilija Date: Tue, 18 Feb 2025 18:14:55 +0100 Subject: [PATCH 8/8] fix merge issues --- pkg/solana/chainreader/batch.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/solana/chainreader/batch.go b/pkg/solana/chainreader/batch.go index b0cd4dbcb..90f4467ae 100644 --- a/pkg/solana/chainreader/batch.go +++ b/pkg/solana/chainreader/batch.go @@ -76,7 +76,7 @@ func doMethodBatchCall(ctx context.Context, lggr logger.Logger, client MultipleA results := make([]batchResultWithErr, len(batch)) // create the list of public keys to fetch - keys := []solana.PublicKey{} + var keys []solana.PublicKey // map batch call index to key index (some calls are event reads and will be handled by a different binding) dataMap := make(map[int]int) @@ -112,7 +112,6 @@ func doMethodBatchCall(ctx context.Context, lggr logger.Logger, client MultipleA } // Fetch the account data - results := make([]batchResultWithErr, len(batch)) data, err := client.GetMultipleAccountData(ctx, keys...) if err != nil { return nil, err