feat: add tx table entry proofs to tx payload proof

[ggutoski]

close #795

• Allow proof generation for zero-length tx payloads. (Previously we disallowed this.)
• Add a trivial from_bytes implementation in preparation for [parent] Support arbitrary payload bytes #747 , which is out of scope for the current PR.
• New TODOs worthy of attention flagged with TODO(795)

[jbearer]

LGTM

[jbearer]

Either in this PR or soon after, we should implement a method something like

impl TxInclusionProof {
  fn validate(&self, payload_comm: VidCommitment, tx: &Transaction) -> bool;
}

This will just help be sure we haven't missed anything in the proof generation that we would need in the verifier. Although with this change I don't think we have (except of course the namespace stuff)

[ggutoski]

Done in 90d8ddd .

The new verify() method has an obnoxious arg list. I left it that way on purpose for now so you can see all the pieces needed to verify a proof. Where should all these pieces go? eg. Should they all become fields of TxInclusionProof?

[ggutoski]

@jbearer Thinking about this a bit more: the problem is that we need both range and payload bytes for three subproofs in a TxInclusionProof. That's a lot of cruft.

I could just add all this info to TxInclusionProof so that the verifier doesn't need to hang on to this info and pass it in a big ugly statement to verify but that would conflict with the design principle we discussed in EspressoSystems/jellyfish#389 (comment) . If it's okay to clean it up here then it should also be okay to clean it up in jellyfish, too.

If we're not careful we could create a cascade of copies of all this metadata at each level of the stack: a copy in the jellyfish proof plus a copy somewhere in the sequencer (maybe in the TxInclusionProof?) to pass to the jellyfish verification, etc.

I'm still not convinced that jellyfish verification should require the range. If we eliminate this requirement from jellyfish then it would eliminate the need to store 3 ranges in the sequencer. Moreover, I think we could include the payload bytes for the tx table length and entry inside TxInclusionProof, which would eliminate the need to store those things separately in the sequencer. I guess my point is that all this cruft is really just implementation details of the proof and should not be exposed to the user. wdyt?

[jbearer]

I think the {table_len,entry_start,entry_end} bytes could all be part of the proof. The reason is that the verifier isn't actually interested in those things, they just want to know generally that the transaction bytes are correct, and those things are implementation details of the proof.

I think we can probably also move tx_payload_range to the proof, since the caller is not likely interested in reading a transaction at any particular offset (how would they know?), they just want to know they have the transaction at the right index. So only tx_index needs to be a verifier input.

Eventually, we may create an even higher-level wrapper for the API endpoint that puts tx_index in the proof as well, since probably the end user queried by hash, and they can check that just by recomputing the hash of tx. But at the level of abstraction we're currently at, I think it makes sense to expose tx_index.

[ggutoski]

Ok sounds good.

I really don't like that the range is duplicated in both the jellyfish proof and in TxInclusionProof and I think I finally have a good solution. We made a mistake including this range in the jellyfish proof. As a general rule, each piece of data should be in the statement or the proof but never both. You have argued that the range should be in the (jellyfish) statement, so it should be removed from the (jellyfish) proof. Indeed, the only use of this data in jellyfish is to check that it equals the range given by the user in the Statement---in other words, it's completely useless.

This is a trivial upstream change that will save a few bytes of duplicated data inside proofs. Make sense?

[jbearer]

Yeah, sound sgood

[jbearer]

I think the {table_len,entry_start,entry_end} bytes could all be part of the proof. The reason is that the verifier isn't actually interested in those things, they just want to know generally that the transaction bytes are correct, and those things are implementation details of the proof.

I think we can probably also move tx_payload_range to the proof, since the caller is not likely interested in reading a transaction at any particular offset (how would they know?), they just want to know they have the transaction at the right index. So only tx_index needs to be a verifier input.

Eventually, we may create an even higher-level wrapper for the API endpoint that puts tx_index in the proof as well, since probably the end user queried by hash, and they can check that just by recomputing the hash of tx. But at the level of abstraction we're currently at, I think it makes sense to expose tx_index.

[ggutoski]

Updated as per comments. verify() is far less ugly now.

Moreover, in 9ce2bd3 I took the liberty to merge 2 payload proofs into 1 for correctness of the 2 entries in the tx table. (They're always adjacent so it's ridiculous to do 2 proofs.)