Consistently use Merkle Tree root as ECChain key

[masih]

Remove the custom ECChain key generation inside gpbft in favor of consistently using Merkle Tree root of chain as key. This key is already used for signature payload generation, and in the new chain exchange as a key to identify a chain. Doing so enables signature validation of messages without having to know the chain.

Consistent use of Merkle Tree root hash as key enables a number of simplifications across the repo:

• unblocks the unification of validation logic between partial and full validators, since caching can use a consistent way to identify messages whether they're partial or not.
• reduce various type gymnastics across root and chain exchange packages by repurposing gpbft ECChain Key.

The work here also introduces lazy-loading for the ECChain, which requires ECChain type to be converted to struct, and be passed by pointer. As part of this work, TipSet is also turned into a pointer consistently across various interfaces.

The ECChain receiver functions are adopted to accommodate potential nil values for a chain.

Fixes #825

[codecov]

Codecov Report

Attention: Patch coverage is 89.28571% with 27 lines in your changes missing coverage. Please review.

Project coverage is 67.26%. Comparing base (5c916d1) to head (dda094f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
partial_validator.go	27.27%	8 Missing ⚠️
chainexchange/pubsub.go	78.26%	4 Missing and 1 partial ⚠️
cbor_gen.go	20.00%	1 Missing and 3 partials ⚠️
gpbft/chain.go	96.11%	2 Missing and 2 partials ⚠️
partial_msg.go	70.00%	3 Missing ⚠️
observer/model.go	0.00%	2 Missing ⚠️
gpbft/gpbft.go	97.61%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #834      +/-   ##
==========================================
- Coverage   67.31%   67.26%   -0.06%     
==========================================
  Files          84       83       -1     
  Lines        9001     9017      +16     
==========================================
+ Hits         6059     6065       +6     
- Misses       2407     2413       +6     
- Partials      535      539       +4     

Files with missing lines	Coverage Δ
certchain/certchain.go	62.81% <100.00%> (ø)
certs/certs.go	92.68% <100.00%> (ø)
consensus_inputs.go	61.87% <100.00%> (-1.26%)	⬇️
emulator/driver_assertions.go	100.00% <100.00%> (ø)
emulator/host.go	77.77% <100.00%> (ø)
emulator/instance.go	89.65% <100.00%> (-4.32%)	⬇️
gpbft/validator.go	91.50% <100.00%> (ø)
host.go	65.09% <100.00%> (+1.18%)	⬆️
merkle/merkle.go	92.30% <ø> (ø)
gpbft/gpbft.go	89.97% <97.61%> (+0.27%)	⬆️
... and 6 more

... and 4 files with indirect coverage changes

[Kubuxu]

I have a slight concern, but it is more general. How do we transition Calibnet to it, given that it lives and breathes the older formats?

[masih]

I have a slight concern, but it is more general. How do we transition Calibnet to it, given that it lives and breathes the older formats?

Good question. If I have not missed anything I think the only wire format change is the ECChain CBOR encoding, where it gets encoded as a struct instead of the old array of TipSets.

To keep compatibility, I can override the CBOR marshalling for it to continue to write in old format since that's all we really need. I actually did this at first then I thought we don't need it having missed the fact that F3 on calibnet is live.

That should be enough right? Is there anything else you can think of that I might have missed?

[Kubuxu]

SGTM

[Kubuxu]

That should be enough right? Is there anything else you can think of that I might have missed?

I think the ECChain is the only thing with persistent data. There is a question of how do we switch to chain exchange scheme but I think allowing it to go down for a bit and catch back up is good enough solution for calibnet.

[masih]

OK I have added compatibility tests with the old format and handcrafted the CBOR decoding for ECChain.

@Kubuxu can I ask you to take a look at c3ac37b before I merge this PR. It's a sensitive part of the code and I'd like another pair of eyes on it please. 🙏

[masih]

I'll hold off resolving conflicts after Kuba had a chance to review the added CBOR encoding.

[Kubuxu]

SGTM, the copy is a bit annoying by oh well