Skip to content

Commit

Permalink
Change account reading err handling and fix get token prices for a no…
Browse files Browse the repository at this point in the history 
…n loop call #1079 (#1078)

* Change CR handling for Addresses not found

* lint

* 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

* fix cr test

* Refactor fix, don't discard account data from accounts we were able to read

* Implement ErrOnMissingAccountData flag for CR calls

* cleanup

* fix merge issues

---------

Co-authored-by: Blaž Hrastnik <[email protected]>
  • Loading branch information
@ilija42 @archseer
ilija42 and archseer authored Feb 20, 2025
1 parent 4b4951a commit 8953ce7
Show file tree
Hide file tree
Showing 8 changed files with 272 additions and 133 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
217 changes: 148 additions & 69 deletions integration-tests/relayinterface/chain_components_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -261,6 +295,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"
Expand All @@ -269,14 +304,50 @@ 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]{
{
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) {
Expand Down Expand Up @@ -767,6 +838,7 @@ func (h *helper) runInitialize(
}

const (
ReadUninitializedPDA = "ReadUninitializedPDA"
MultiRead = "MultiRead"
ReadWithAddressHardCodedIntoResponse = "ReadWithAddressHardCodedIntoResponse"
MultiReadWithParamsReuse = "MultiReadWithParamsReuse"
Expand Down Expand Up @@ -848,6 +920,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",
Expand Down
32 changes: 18 additions & 14 deletions pkg/solana/chainreader/batch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (

"github.com/gagliardetto/solana-go"

"github.com/smartcontractkit/chainlink-common/pkg/logger"
"github.com/smartcontractkit/chainlink-common/pkg/types"
"github.com/smartcontractkit/chainlink-common/pkg/values"
)

type call struct {
Namespace, ReadName string
Params, ReturnVal any
Namespace, ReadName string
Params, ReturnVal any
ErrOnMissingAccountData bool
}

type batchResultWithErr struct {
Expand All @@ -33,20 +35,21 @@ 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{
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
}
}

results, err := doMethodBatchCall(ctx, client, bdRegistry, batch)
results, err := doMethodBatchCall(ctx, lggr, client, bdRegistry, batch)
if err != nil {
return err
}
Expand All @@ -69,15 +72,14 @@ 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) {
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)

for idx, batchCall := range batch {
rBinding, err := bdRegistry.GetReader(batchCall.Namespace, batchCall.ReadName)
if err != nil {
Expand Down Expand Up @@ -129,16 +131,18 @@ func doMethodBatchCall(ctx context.Context, client MultipleAccountGetter, bdRegi
returnVal: batchCall.ReturnVal,
}

if data[dataIdx] == nil || len(data[dataIdx]) == 0 {
results[idx].err = ErrMissingAccountData

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", keys[dataIdx].String())
continue
}

rBinding, err := bdRegistry.GetReader(results[idx].namespace, results[idx].readName)
if err != nil {
results[idx].err = err

continue
}

Expand Down
Loading

0 comments on commit 8953ce7

Please sign in to comment.