feat(protocol-parser): add keySchema/valueSchema helpers #1443
Conversation
…Schema/valueSchema
🦋 Changeset detectedLatest commit: f2be76b The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| keySchema: TSchema, | ||
| data: readonly Hex[] | ||
| ): SchemaToPrimitives<TSchema> { | ||
| // TODO: refactor and move all decodeKeyTuple logic into this method so we can delete decodeKeyTuple |
There was a problem hiding this comment.
is decodeKeyTuple still used somewhere else? If not, should we just move the logic in there in this PR?
There was a problem hiding this comment.
it's used by blockLogsToStorage which is getting refactored in the other PR
didn't wanna refactor everything quite yet to avoid a big rebase of the other PR, just wanted to add new methods and deprecate old ones for an easy first pass
There was a problem hiding this comment.
updated the original issue to remind me to refactor this: #1296
|
|
||
| it("can decode an out of bounds array", () => { | ||
| const schema = { staticFields: [], dynamicFields: ["uint32[]"] } as const; | ||
| const values = decodeRecord(schema, "0x0000000000000000000000000000000000000000000000000400000000000004"); |
There was a problem hiding this comment.
The hex represents an encoded dynamic length field with total length 4 and length of the first array of 4, correct? Can you add more context for why the decoded array has one element with value 0? Somehow I would have expected either an empty because there is no data after the encoded data length, or an array with 4 elements that are all 0?
There was a problem hiding this comment.
I think I had added this test for proof that padSliceHex was doing the right thing internally when used within decodeRecord. I can't recall where exactly I was getting a the hex value but I copied it from some tests or logs that were failing without padSliceHex.
To explain what's going on:
The total length and first array length is the byte length (4 bytes in the case of uint32). Meaning that there's one item/value for the uint32[] field.
The record hex is "trimmed" to the right-most byte (end of the counter). On chain, any bytes after the right-most byte would be treated as zeros, so we want to have the same behavior in the client.
So if we were to read the same value on chain, we should get a value of [0] for this record (one uint32 value with unset bytes, i.e. 0). This test ensures that the same behavior happens in the client.
There was a problem hiding this comment.
Ahh makes sense, had forgotten that the length corresponds to bytes and assumed elements. Thanks for the explanation!
| value.map(() => staticFieldType), | ||
| value | ||
| ); | ||
| // TODO: we can remove conditional once this is fixed: https://github.com/wagmi-dev/viem/pull/1147 |
There was a problem hiding this comment.
seems like it got merged already!
There was a problem hiding this comment.
yep! we'll need to bump viem to get this change so gonna leave this here for now
| @@ -0,0 +1,8 @@ | |||
| import { Hex } from "viem"; | |||
|
|
|||
| export function padSliceHex(data: Hex, start: number, end?: number): Hex { | |||
There was a problem hiding this comment.
should we add a small typedoc comment with something like "slices the hex string while padding data out of bound data with 0; including start index, excluding end index"?
There was a problem hiding this comment.
done! also renamed to readHex because I didn't think padSliceHex was clear enough (felt more like it was named after the implementation rather than the intent)
Co-authored-by: alvarius <[email protected]>
related to #1296
pulled out of #1354