feat(protocol-parser): add encoding/decoding functions

[holic]

Moves schemas to a class instance with encoding/decoding functions. Pulled out of #1033 for #1075.

cc @yonadaaa

[changeset-bot]

⚠️ No Changeset found

Latest commit: 05f211b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[alvrs]

i believe there can also just be dynamic fields? are we even enforcing that there are any fields?

[holic]

oh I think you might be right, will remove the comment

would be good to have tests for no-static-fields and no-dynamic-fields though

[alvrs]

Code looks good in general, but one thing I wonder about is whether we want to make Schema a class or to go the factory / utils route. It seems like the js ecosystem is generally going in the direction of avoiding classes (see this opinion piece or viem) and most of our codebase is using functions over classes too. Wonder what your opinion is here!

[alvrs]

what does this TODO mean? is the refactor to make bytesOffset a bigint instead of a number? is there a downside to already do that now?

[holic]

I think since this is new code, I can probably do that now. This was copied over from another area of the codebase, where it was a much bigger refactor.

[holic]

ahh I remember now: viem's sliceHex operations expect a number, so that's why bytesOffset is a number

moving it to bigint means that I can't use sliceHex unless downcasting, which has the same potential issue.

[holic]

uint40 max is 1099511627775, which means we'd need a 1TB string to hit this issue. Feels like we could punt on this for now.

[alvrs]

oh nice, we need this for #1074 too!

[holic]

whether we want to make Schema a class or to go the factory / utils route

Yeah I keep going back and forth on this!

const schema = createSchema(staticFields, dynamicFields); // or just {staticFields, dynamicFields}
const schema = hexToSchema(hex);
const data = encodeRecord(schema, [1, 2, 3, 'something']);
// versus
const schema = new Schema(staticFields, dynamicFields);
const schema = Schema.fromHex(staticFields, dynamicFields);
const data = schema.encodeRecord([1, 2, 3, 'something']);

and

const tableId = createTableId(namespace, name); // or just {namespace, name}
const tableId = hexToTableId(hex);
const tableIdString = tableIdToString(tableId);
const tableIdHex = tableIdToHex(tableId);
// versus
const tableId = new TableId(namespace, name);
const tableId = TableId.fromHex(hex);
const tableIdString = tableId.toString();
const tableIdHex = tableId.toHex();

(one small perk of the class approach is that .toString() is called automatically when string interpolated, so we can kinda choose whatever format we want for debugging, etc., but maybe it's better to be more explicit about the kind of string you want based on the context)

[alvrs]

const schema = createSchema(staticFields, dynamicFields); // or just {staticFields, dynamicFields}
const schema = hexToSchema(hex);
const data = encodeRecord(schema, [1, 2, 3, 'something']);
// versus
const schema = new Schema(staticFields, dynamicFields);
const schema = Schema.fromHex(staticFields, dynamicFields);
const data = schema.encodeRecord([1, 2, 3, 'something']);

I don't have a strong opinion yet, but i do find the first approach more aesthetically pleasing, and it's not even really more to type. And when googling javascript class vs function I don't find any opinion pieces advocating for classes, only those advocating for functions, eg https://everyday.codes/javascript/please-stop-using-classes-in-javascript/ 😄

[alvrs]

lgtm!