Use ChainHash instead of BlockHash as applicable

[jkczyz]

ChainHash is more appropriate for places where an arbitrary BlockHash is not desirable. This type was introduced in later versions of the bitcoin crate, thus BlockHash was used instead.

Using ChainHash also makes it easier to check if ChannelManager is compatible with an Offer.

[codecov-commenter]

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (b1d3aa8) 89.06% compared to head (54f96ef) 89.06%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2662   +/-   ##
=======================================
  Coverage   89.06%   89.06%           
=======================================
  Files         112      112           
  Lines       87417    87417           
  Branches    87417    87417           
=======================================
+ Hits        77856    77861    +5     
+ Misses       7322     7320    -2     
+ Partials     2239     2236    -3     

Files	Coverage Δ
lightning-net-tokio/src/lib.rs	75.00% <100.00%> (+0.63%)	⬆️
lightning/src/ln/channel.rs	88.37% <100.00%> (ø)
lightning/src/ln/functional_tests.rs	97.36% <100.00%> (+0.01%)	⬆️
lightning/src/ln/onion_route_tests.rs	97.56% <100.00%> (+<0.01%)	⬆️
lightning/src/ln/priv_short_conf_tests.rs	97.46% <100.00%> (ø)
lightning/src/routing/router.rs	94.07% <100.00%> (ø)
lightning/src/routing/scoring.rs	91.93% <100.00%> (ø)
lightning/src/routing/test_utils.rs	99.33% <100.00%> (ø)
lightning/src/routing/utxo.rs	89.47% <100.00%> (ø)
lightning/src/util/scid_utils.rs	98.87% <100.00%> (-0.01%)	⬇️
... and 7 more

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jkczyz]

@TheBlueMatt I'm at a loss as to why the full stack fuzz test uses a seemingly incorrect chain hash of 0000000000000000000000000000000000000000000000000000000000000075:

rust-lightning/fuzz/src/full_stack.rs

Line 775 in 2c51080

// 0020 7500000000000000000000000000000000000000000000000000000000000000 ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb181909679 000000000000c350 0000000000000000 0000000000000162 ffffffffffffffff 0000000000000222 0000000000000000 000000fd 0006 01e3 030000000000000000000000000000000000000000000000000000000000000001 030000000000000000000000000000000000000000000000000000000000000002 030000000000000000000000000000000000000000000000000000000000000003 030000000000000000000000000000000000000000000000000000000000000004 - beginning of open_channel message

But when initializing ChannelManager with Network::Bitcoin, the genesis_hash is indeed that when examining with dbg!.

rust-lightning/fuzz/src/full_stack.rs

Lines 459 to 464 in 2c51080

	let network = Network::Bitcoin;
	let best_block_timestamp = genesis_block(network).header.time;
	let params = ChainParameters {
	network,
	best_block: BestBlock::from_network(network),
	};

rust-lightning/lightning/src/ln/channelmanager.rs

Line 2261 in 2c51080

genesis_hash: genesis_block(params.network).header.block_hash(),

However, it is the expected 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000 in normal tests. Is there something about the fuzz testing setup that gives a different block hash for the genesis block? While debugging, the merkle root looks glaringly wrong as 000000000000000000000000000000000000000000000000000000000000001b.

[/Users/jkczyz/src/rust-lightning/lightning/src/ln/channelmanager.rs:2261] params.network = Bitcoin
[/Users/jkczyz/src/rust-lightning/lightning/src/ln/channelmanager.rs:2261] genesis_block(dbg!(params.network)) = Block {
    header: BlockHeader {
        version: 1,
        prev_blockhash: 0000000000000000000000000000000000000000000000000000000000000000,
        merkle_root: 000000000000000000000000000000000000000000000000000000000000001b,
        time: 1231006505,
        bits: 486604799,
        nonce: 2083236893,
    },
    txdata: [
        Transaction {
            version: 1,
            lock_time: PackedLockTime(
                0,
            ),
            input: [
                TxIn {
                    previous_output: OutPoint {
                        txid: 0000000000000000000000000000000000000000000000000000000000000000,
                        vout: 4294967295,
                    },
                    script_sig: Script(OP_PUSHBYTES_4 ffff001d OP_PUSHBYTES_1 04 OP_PUSHBYTES_69 5468652054696d65732030332f4a616e2f32303039204368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73),
                    sequence: Sequence(
                        4294967295,
                    ),
                    witness: Witness {
                        content: [],
                        witness_elements: 0,
                        last: 0,
                        second_to_last: 0,
                    },
                },
            ],
            output: [
                TxOut {
                    value: 5000000000,
                    script_pubkey: Script(OP_PUSHBYTES_65 04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f OP_CHECKSIG),
                },
            ],
        },
    ],
}
[/Users/jkczyz/src/rust-lightning/lightning/src/ln/channelmanager.rs:2261] dbg!(genesis_block(dbg! (params.network))).header.block_hash() = 0000000000000000000000000000000000000000000000000000000000000075

[TheBlueMatt]

Yes, the fuzz environment makes sha256 a simple xor of the input bytes, so block hashes are calculated totally differently. Thus, the previous code that actually hashed the genesis block got that value, but now we're fetching from the hard-coded hash in rust-bitcoin, so we need the real value. If you just swap it out it WFM.

[jkczyz]

Ah, thanks! Should be working now.

[TheBlueMatt]

CI is quite sad, may need a rebase.

[valentinewallace]

LGTM after CI fix.

[dunxen]

Cool, I see what's going on in the msg tests that were updated.

LGTM

[jkczyz]

Rebased