Build the commitment without bincode #2194
Conversation
ImJeremyHe
requested review from
philippecamacho,
sveitser,
jbearer,
tbro and
imabdulbasit
as code owners
ImJeremyHe
force-pushed
the
jh/commitment
branch
2 times, most recently
from
5cb0788
to
0bf3a58
Compare
| fn tag() -> String { | ||
| "BID_TX".to_string() | ||
| } | ||
|
|
||
| fn commit(&self) -> Commitment<Self> { | ||
| let comm = committable::RawCommitmentBuilder::new(&Self::tag()) | ||
| .field("body", self.body.commit()) |
There was a problem hiding this comment.
I think it was probably a previous mistake to have an explicit commitment for BidTxBody. The BidTx can just access body methods (or fields) directly in its own commitment. Having a body is a structural necessity for signing but behavior should act as if all the fields are on BidTx.
There was a problem hiding this comment.
Do you mean we don't need to implemnt the commitable for the BodyTxBody? But its commit function is used here
And BodyTx has an extra field signature. I am not sure if we need to build the commitment of BodyTx with the signature field. But since it is shown in the header, I think it's better to build the commitment with it.
| fn tag() -> String { | ||
| "BID_TX_BODY".to_string() | ||
| } | ||
|
|
There was a problem hiding this comment.
It doesn't appear in the diff, but on line 57 there is another call to bincode. We should remove this one as well, right?
ImJeremyHe
force-pushed
the
jh/commitment
branch
from
323a8b1
to
2c0c642
Compare
|
@tbro I wonder which version will be used on our mainnet? v0.2? |
This PR builds the commitment of v0.3 header without bincode, which helps other language be align with the Rust code.
This PR:
Committabletrait for some types required in v0.3 Headercommitin v0.3 HeaderThis PR does not:
Relevant PR:
EspressoSystems/espresso-sequencer-go#52