search

LayerTwo-Labs / cusf_sidechain_proto

gRPC protocol definitions for a CUSF BIP300/BIP301 sidechain.
1 stars 2 forks source link

mainchain/wallet: use ReverseHex for BMM critical data hashes #11

Closed torkelrogstad closed 6 days ago

Ash-L2L commented 1 week ago

I'm not sure about this one. Critical hashes originate from sidechains that probably do not use reverse-hex encoding when displaying hashes. And prev_bytes is not even a hash, so it seems odd to use an encoding that Bitcoin only uses for Sha256d hashes.

torkelrogstad commented 1 week ago

Agreed on the critical hashes. But prev_bytes is a hash, isn't it? From the BIP: prevMainBlock (the blockhash of the previous main:block).

https://github.com/bitcoin/bips/blob/master/bip-0301.mediawiki#bmm-request

In the legacy C++ implementation (which is what I'm basing this off of) it is compared against the MC chain tip: https://github.com/LayerTwo-Labs/mainchain/blob/05e71917042132202248c0c917f8ef120a2a5251/src/wallet/rpcwallet.cpp#L3928-L3936

nchashch commented 1 week ago

The current specification for BIP301 is:

https://github.com/LayerTwo-Labs/bip300_bip301_specifications/blob/master/bip301.md

the legacy C++ implementation shouldn't be used as a reference.

nchashch commented 1 week ago

So if I understand this correctly, the point of this change is to allow the client to "paste in" the return value of Bitcoin Core getblockhash RPC, right?

Because Bitcoin block hashes are encoded in ReverseHex format.

If we use ConsensusHex, then the client would have to do an extra step of converting it before making the request.

torkelrogstad commented 1 week ago

So if I understand this correctly, the point of this change is to allow the client to "paste in" the return value of Bitcoin Core getblockhash RPC, right?

Yes, that's pretty much correct. Plus other sources of blockchain data, such as mempool.drivechain.live and so on