celestiaorg / go-fraud

Generic p2p fraud proof library
Apache License 2.0
10 stars 7 forks source link

fraud: remove block height from proof and make all fields on proof private #7

Open vgonkivs opened 2 years ago

vgonkivs commented 2 years ago

We should remove BlockHeight from Proof structure and interface as it was used to fetch header using GetBlockByHeight method. in celestiaorg/celestia-node#827 the Proof structure was extended with BlockHash. So now we can replace GetBlockByHeight with GetBlockHash and remove BlockHeight field at all

renaynay commented 2 years ago

What is the rationale for using hash over height?

vgonkivs commented 2 years ago

Because height is not a unique number. In case, when BEFP is created, user should restart the node from previous block and will get a new block at known height. That's the reason why I added block hash. As we now store hash we can get a block by its hash and block height becomes redundant.

adlerjohn commented 2 years ago

Re-orgs aren't possible under Tendermint, and under a weak subjectivity assumptions you can ignore everything that's not on the subjective canonical chain.

That being said, using hash can be better for things like proving that some orphaned block was invalid (e.g. to back-rationale a hard fork), as height alone doesn't identify that block. Would it be better to include both height and hash?

Wondertan commented 1 year ago

We include. I am not sure there is a benefit to having two types of pointers to a block, thus the issue to remove one of them, particularly the height pointer.