chore: rewrite block payload

[ggutoski]

Closes #1076

DRAFT MODE - PR DESCRIPTION TODO

Testing CI...

This PR:

This PR does not:

Key places to review:

[sveitser]

I have only one failure locally:

failures:
    message_compat_tests::test_message_compat

This is the diff

diff data/messages-actual.json data/messages.json 
11c11
<                 "builder_commitment": "BUILDER_COMMITMENT~_FlsTMQYXvjU_M1TpJFyrGe9BGPU9Di-qALdKJb2hegZ",
---
>                 "builder_commitment": "BUILDER_COMMITMENT~tEvs0rxqOiMCvfe2R0omNNaphSlUiEDrb2q0IZpRcgA_",
32c32,34
<                 "ns_table": "AAAAAA==",
---
>                 "ns_table": {
>                   "bytes": "AAAAAA=="
>                 },
320c322,324
<               "metadata": "AQAAAAEAAAALAAAA",
---
>               "metadata": {
>                 "bytes": "AQAAAAEAAAALAAAA"
>               },

[ggutoski]

I have only one failure locally:

Yep I just found that one, too. Took me a while to dig it out of the logs. I'll fix that one shortly. Thank you!! ❤️

[philippecamacho]

A possible exception: NamespaceId is an identifier, not a quantity. In the future we might use 32 bytes for NamespaceId to allow a namespace ID to be a hash output. In that case NamespaceId will need modification and can no longer use the functions in uint_bytes.rs.

I understand that while currently the set of validators is permissioned, the set of rollups is not. Can we still use u64 for NamespaceId?

[ggutoski]

Commit https://github.com/EspressoSystems/espresso-sequencer/pull/1499/commits/b593a96adb42ea41bf4321fd5945078c7fefc64b reverts hotfix PR #1504, which is no longer necessary. But I don't know where to put a test.

[EDIT: I already have a test that uses 64-bit namespace IDs.] https://github.com/EspressoSystems/espresso-sequencer/blob/b593a96adb42ea41bf4321fd5945078c7fefc64b/sequencer/src/block/test.rs#L161

Note that API/serialization for NamespaceId accepts any u64. Thus, the sequencer in its current form will crash if the constant NS_ID_BYTE_LEN is anything different from 8.

This PR does not touch NamespaceId but perhaps it should. Unfortunately, a proper fix requires that NamespaceId serialization code has visibility of NS_ID_BYTE_LEN, which I'd very much like to keep private to ns_table.rs.

One solution is to move NamespaceId struct into ns_table.rs. I like this solution because NamespaceId depends on the low-level binary details of a payload, much like similar new structs such as NsIndex, TxIndex, NumTxs, etc.

~~I'll let this idea sit overnight and gather opinions. If I don't hear anything by tomorrow then I'll move NamespaceId to ns_table.rs as part of this PR.~~

[EDIT: On second thought that's a significant enough change that it should go into a separate PR.]

[ggutoski]

I understand that while currently the set of validators is permissioned, the set of rollups is not. Can we still use u64 for NamespaceId?

I don't understand the concern here. What could a malicious rollup do to exploit a 8-byte namespace ID?

[philippecamacho]

I understand that while currently the set of validators is permissioned, the set of rollups is not. Can we still use u64 for NamespaceId?

I don't understand the concern here. What could a malicious rollup do to exploit a 8-byte namespace ID?

This is not a security concern, rather I wonder how the short ID will be assigned to rollups in practice given that any rollup can join?

[philippecamacho]

A general question: do we need to do something special in order to handle possible changes regarding how namespaces, payloads will be serialized in the future, or is this problem already handled by a general solution we have? cc @nyospe @jbearer

[ggutoski]

I understand that while currently the set of validators is permissioned, the set of rollups is not. Can we still use u64 for NamespaceId?

I don't understand the concern here. What could a malicious rollup do to exploit a 8-byte namespace ID?

This is not a security concern, rather I wonder how the short ID will be assigned to rollups in practice given that any rollup can join?

I believe the concern is not that we'll run out of 64-bit chain IDs. Perhaps you're concerned that different rollups might accidentally collide at a single namespace ID? If rollups choose their namespace IDs at random then the probability of collision is acceptably small. If instead rollups choose their namespace IDs maliciously then we have the same problem whether the chain ID is 8 bytes or 32 bytes.

My understanding is that precedent for EVM chains is that each chain simply chooses its own chain ID so as not to conflict with any other known chain. I suspect all parties are incentivized to avoid a chain ID conflict, and so we never hear of any problem caused by it. There's even a public list of EMV chain IDs: https://chainlist.org/ It seems that EVM chain IDs are also 64 bits.

I expect a similar dynamic to hold for namespaces in our setting. tldr; I don't think it's a problem.

[philippecamacho]

I understand that while currently the set of validators is permissioned, the set of rollups is not. Can we still use u64 for NamespaceId?

I don't understand the concern here. What could a malicious rollup do to exploit a 8-byte namespace ID?

This is not a security concern, rather I wonder how the short ID will be assigned to rollups in practice given that any rollup can join?

I believe the concern is not that we'll run out of 64-bit chain IDs. Perhaps you're concerned that different rollups might accidentally collide at a single namespace ID? If rollups choose their namespace IDs at random then the probability of collision is acceptably small. If instead rollups choose their namespace IDs maliciously then we have the same problem whether the chain ID is 8 bytes or 32 bytes.

My understanding is that precedent for EVM chains is that each chain simply chooses its own chain ID so as not to conflict with any other known chain. I suspect all parties are incentivized to avoid a chain ID conflict, and so we never hear of any problem caused by it. There's even a public list of EMV chain IDs: https://chainlist.org/ It seems that EVM chain IDs are also 64 bits.

I expect a similar dynamic to hold for namespaces in our setting. tldr; I don't think it's a problem.

Concern is probably too strong of a word :). I just wonder if we should create a task for letting the rollups define their namespace ID (something similar to chainlist.org you mentioned) or if this is already covered for Decaf.

[ggutoski]

A general question: do we need to do something special in order to handle possible changes regarding how namespaces, payloads will be serialized in the future, or is this problem already handled by a general solution we have? cc @nyospe @jbearer

@philippecamacho I don't have a complete answer to this question but I can tell you that we have tests that catch any breaking changes to serialization in reference_tests.rs, message_compat_tests.rs. (Note that the present PR makes breaking changes, so I've updated these tests accordingly.)

[ggutoski]

On further discussion with @jbearer , I need to investigate whether it's feasible to maintain serialization compatibility. Putting this PR back into draft mode for now.