Nested Value Codec Access

[EasterTheBunny]

This commit introduces the ability to use a codec to encode/decode individual fields of a struct, including traversal to nested structs.

NONEVM-1063

Dismissing due to test failure

[reductionista]

You may have already realized this, but unfortunately this doesn't work. You can't use a codec for a subfield to decode raw, since raw is the entire encoded struct, not just one subfield.

I wish this had occurred to me on Friday, but I think there's a fundamental problem with this interface--of having CreateType return only the subfield type and then passing that to Decode.

Stepping back a moment: The only reason we need LogPoller to decode the raw data at all is because, unlike with EVM-encoded data, there's no way to predict what offset into a Borsh-encoded struct a subfield will be at without Borsh-decoding the entire struct.

So whatever we do to solve this problem, we have to decode the whole struct--not just a subfield. Because Decode doesn't have type information for the whole struct, it can't do that. So returning only the subfield type from CreateType is not useful.

We could fix this by having CreateType return the type of the entire struct, and passing that to Decode. Then having Decode decode the whole struct, and then extract the requested element out of it based on the itemType path. But that's an unnecessarily convoluted way to solve a much simpler problem.

The first two steps of what we need to solve it in the most straightforward way are already working without this PR:

1. Pass eventType as itemType to CreateType, and have it return the type of the whole struct.
2. Pass that into Decode and have that return the decoded struct.

All we need that's new is the third step:
3. A function which takes a struct as input and returns a nested subfield of that struct based on a dot-separated path.

This is what I originally implemented using reflect, and could probably dig up again. I'm leaning now towards that approach, which wouldn't require modifying the codec... since doing this part doesn't require any knowledge of registered types or codecs, and doesn't involve any translation from on chain to off chain or vice versa. It still seems like something the codec package might be able to do more easily than writing it from scratch, but if so I have no idea how.

[EasterTheBunny]

Yes, this only works if you want to encode/decode a single field. It was never expected to be able to extract a single field from a fully encoded struct without encoding that single field first.

Example:

type TypeA struct {
    A int
    B string
}

You can decode the whole struct with Decode(ctx, allData, &structInstance, "") where allData is the encoded value of the entire struct. However, you cannot do Decode(allData, &fieldA, "A") and get only the int data.

Alternatively you can do something like the following:

// this would be the entire encoded struct
var allData []byte

var structInstance TypeA
_ = theCodec.Decode(allData, &structInstance, "TypeA")

// this will encode the value of the B field
strBts, _ := theCodec.Encode(ctx, structInstance.B, "TypeA.B")

I would envision in LogPoller that you could have different encoding and decoding codecs where you want to decode a struct and encode a single field, but use different encodings for each.

[amit-momin]

Just wanted to mention that Silas wrote something that could be used for step 3 for CW here. If we go this route, it might be worth pulling it into a util file so we can use it for CR too.

[reductionista]

@amit-momin Very interesting! Yes, that seems pretty similar to the helper I had to implement... we should def look at combining them when we get a chance. This is the one I needed to get everything working:
https://github.com/smartcontractkit/chainlink-solana/pull/1002/files#diff-43599bcdcf8c088e3eac4f93fd79202871475fcdeb2dcc9494a8935b9f991a07R439-R491

[reductionista]

// this would be the entire encoded struct
var allData []byte

var structInstance TypeA
_ = theCodec.Decode(allData, &structInstance, "TypeA")

// this will encode the value of the B field
strBts, _ := theCodec.Encode(ctx, structInstance.B, "TypeA.B")

That's very different from anything LogPoller does. Even if we completely redesigned it to work this way, it wouldn't be able to work. The reason we're decoding the data at all is because we need to know the offset of where the subkeys are in order to search for them in queries... if you don't know the offset, then any decoding or encoding you do of it (in place) is pointless--either way there is no way to search for it. There would also be no way to build an index on something like that, since every subkey would be at a different offset. That's why it has to extract them and index them separately in their own column.

[reductionista]

However, you cannot do Decode(allData, &fieldA, "A") and get only the int data.

FWIW, this part actually did work. The test case I used is the same as what you have: it was able to decode A successfully, but only because it was the first element in the struct so the offset happened to be 0. That's why I thought it was working at one point. It's passing "TypeA.B" as the itemType that failed. It couldn't decode the string because it thought the string started at the beginning of the data representing the encoded data instead of the middle. So it interpreted the integer A as a string length instead of a value!

[ilija42]

The general approach looks valid, I'll do a more thorough review after modifiers are done

[ilija42]

nit

Suggested change

	type extendedItemType string
	// extendedItemType is used for parsing through nested item types
	type extendedItemType string

[EasterTheBunny]

I ended up making this an exported function named ItemTyper.

[ilija42]

To allow access to nested fields, this function applies no modifications if a modifier by the specified name is not found.

This comment makes it sound as if the func won't throw an error if a modifier is not found

[EasterTheBunny]

I updated this comment to have more clarity.

[ilija42]

Why is it now okay for the modifier to not exist?

[EasterTheBunny]

I changed this to the way it was to maintain backward compatibility.

[ilija42]

maybe add a length check to prevent panic, although these should always be the same size

[ilija42]

Suggested change

	// if itemType wasn't referencing a nested field

[ilija42]

wdyt about adding a test that checks if a proper FieldCodec has been retrieved