Adjust get_block RPC to match Bitcoin Core output

[moisesPompilio]

What is the purpose of this pull request?

• Bug fix
• Documentation update
• New feature
• Test
• Other:

Which crates are being modified?

• floresta-chain
• floresta-common
• floresta-compact-filters
• floresta-electrum
• floresta-node
• floresta-rpc
• floresta-watch-only
• floresta-wire
• bin/florestad
• bin/floresta-cli
• Other:

Description and Notes

Adjusted the get_block RPC output to match Bitcoin Core’s behavior.
Changes include:

• Fixed strippedsize to correctly exclude witness data.
• Ensured bits, chainwork, and version_hex fields use big-endian hexadecimal encoding.
• Updated difficulty calculation for accurate floating-point results.
• Added the missing target field returning the block target in big-endian hexadecimal.

How to verify the changes you have done?

The changes can be verified by comparing the get_block RPC output from Floresta with Bitcoin Core’s reference output and documentation.
For the genesis block (0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206), Bitcoin Core returns:

{
  "hash": "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206",
  "confirmations": 1,
  "height": 0,
  "version": 1,
  "versionHex": "00000001",
  "merkleroot": "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b",
  "time": 1296688602,
  "mediantime": 1296688602,
  "nonce": 2,
  "bits": "207fffff",
  "target": "7fffff0000000000000000000000000000000000000000000000000000000000",
  "difficulty": 4.656542373906925e-10,
  "chainwork": "0000000000000000000000000000000000000000000000000000000000000002",
  "nTx": 1,
  "nextblockhash": "5a31f5b914660757c24e62459b8905f64c7b9ee64740da333eaf8372aed81c82",
  "strippedsize": 285,
  "size": 285,
  "weight": 1140,
  "tx": [
    "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b"
  ]
}

The reference behavior and expected field serialization can be cross-checked directly in Bitcoin Core’s implementation
src/rpc/blockchain.cpp and its official RPC documentation.

[jaoleal]

LGTM, I still need to check that everything is correct but overall it looks good.

[jaoleal]

@Davidson-Souza can you approve CI ?

[Davidson-Souza]

@moisesPompilio this needs rebase

[Davidson-Souza]

I've tested this on block 00000000000000000000fa2dc17a14860712e9c852e9cbf4c5a61b5df25b94d2 and the MTP is different:

Core: 1763994343
Floresta: 1763994302

[moisesPompilio]

I've tested this on block 00000000000000000000fa2dc17a14860712e9c852e9cbf4c5a61b5df25b94d2 and the MTP is different:

Core: 1763994343 Floresta: 1763994302

I adjusted the calculation based on what Bitcoin Core does, and the tests now produce the correct values.

[Davidson-Souza]

I've been thinking about how to expose things like MTP and chainwork in a reusable way, since we'll use it in other places. The best way I thought is an extended trait implemented for Block that takes a BlockchainInterface as argument. So we would have something like:

impl BlockExt {
   fn mtp(&self, chain: &impl BlockchainInterface);

   fn chain_work(&self, chain: &impl BlockchainInterface); 

   fn alt_tips(&self, chain: &impl BlockchainInterface);
  
  // what else??
}

This way we don't pollute BlockchainInterface too much. What do you guys think? (This is not supposed to be implemented here, just thinking out loud)

[moisesPompilio]

I've been thinking about how to expose things like MTP and chainwork in a reusable way, since we'll use it in other places. The best way I thought is an extended trait implemented for Block that takes a BlockchainInterface as argument. So we would have something like:

impl BlockExt {
   fn mtp(&self, chain: &impl BlockchainInterface);

   fn chain_work(&self, chain: &impl BlockchainInterface); 

   fn alt_tips(&self, chain: &impl BlockchainInterface);
  
  // what else??
}

This way we don't pollute BlockchainInterface too much. What do you guys think? (This is not supposed to be implemented here, just thinking out loud)

It makes sense. Using Header instead of Block seems more appropriate, since these functions only need header data. That would also let getblockheader use them without requiring a full Block. Should we open an issue to handle this more cleanly, or address it directly in this PR?

[Davidson-Souza]

Using Header instead of Block seems more appropriate, since these functions only need header data. That would also let getblockheader use them without requiring a full Block

Right!

Should we open an issue to handle this more cleanly, or address it directly in this PR?

I think we can do it in another PR

[moisesPompilio]

bf1a571 I added documentation for the getblock RPC and adjusted the verbosity default value to 1. Verbosity is no longer required, because in Bitcoin Core's documentation, verbosity is optional and the default value is 1.

[Davidson-Souza]

ACK 1df8140

[jaoleal]

LGTM but it needs some changes...

sry for the late response

[moisesPompilio]

ce45338 I fixed a problem in the getblock.py test during sync, it was failing because Floresta wasn't able to finish downloading all blocks. I just moved block generation to before the peers connect. I also added a new test feature in getblock.py, which is the ability to check by block height instead of by the last known block hash. So I added a test target that picks a random block to compare. Now the test compares the genesis block, the last block in the chain, and a random block.

[jaoleal]

ACK ce45338

[Davidson-Souza]

ACK ce45338