Create fresh destination/shutdown scripts for each channel in KeysManager

[TheBlueMatt]

The trait tells us to, for privacy, but we ignore it. While we're at it, we should probably merge destination + shutdown scripts cause they don't need to be separate. When we do this we need to ensure we bump some serialization version so that old clients don't read new KeysManager and not know about keys.

[vss96]

I had a look at the keysinterface and had two questions:

1. In the implementation for the Keysinterface for KeysManager, the functions returning the destination and shutdown script return a clone of the script. Does a fresh script mean using the pubkey to recreate the scripts again via the p2wsh or p2wpkh methods?
2. What does it mean to merge two different scripts? How do you hold information about two different scripts in one? From what I see some of the methods do have some assertions on the shutdown_pubkey, so if we did have a single merged script how would I generate the required shutdown key to replace the same assertion?

[TheBlueMatt]

In the implementation for the Keysinterface for KeysManager, the functions returning the destination and shutdown script return a clone of the script. Does a fresh script mean using the pubkey to recreate the scripts again via the p2wsh or p2wpkh methods?

No, it would mean creating a whole new script using some deterministic derivation path.

What does it mean to merge two different scripts? How do you hold information about two different scripts in one? From what I see some of the methods do have some assertions on the shutdown_pubkey, so if we did have a single merged script how would I generate the required shutdown key to replace the same assertion?

As in, just use the same script in both places.

[vss96]

No, it would mean creating a whole new script using some deterministic derivation path.

Need some context here 😓

[TheBlueMatt]

Oops I'm so sorry I managed to let this fall off my radar.

To create a new public key, look at how we derive keys from the master now, and do something similar, except likely a second layer HD path (ie derive a key and then use that as a new master). That way we can just extend that HD path arbitrarily and create as many pubkeys as we want. Happy to point to specific code if you need, just let me know.

[vss96]

@TheBlueMatt i guess this is what you meant? (new method from KeysManager)

			Ok(master_key) => {
				let node_secret = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(0).unwrap()).expect("Your RNG is busted").private_key.key;
				let destination_script = match master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(1).unwrap()) {
					Ok(destination_key) => {
						let wpubkey_hash = WPubkeyHash::hash(&ExtendedPubKey::from_private(&secp_ctx, &destination_key).public_key.to_bytes());
						Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
						              .push_slice(&wpubkey_hash.into_inner())
						              .into_script()
					},
					Err(_) => panic!("Your RNG is busted"),
				};
				let shutdown_pubkey = match master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(2).unwrap()) {
					Ok(shutdown_key) => ExtendedPubKey::from_private(&secp_ctx, &shutdown_key).public_key.key,
					Err(_) => panic!("Your RNG is busted"),
				};

[TheBlueMatt]

Right, we should drop the destination_script logic, take the shutdown_pubkey and keep it as an ExtendedPubKey, deriving new children and then building both a destination and shutdown pubkey with it per-channel.

[vss96]

@TheBlueMatt In the last part of spend_spendable_outputs, we get the derivation_idx by comparing destination_script and output.script_pubkey. How do I use the ExtendedPubKey to create the destination_script? I see that there is a method called ckd_pub to derive public child key but the secp_ctx that it requires(Verification trait) is not available in this particular function.

[TheBlueMatt]

I think we should include the channel_keys_id in SpendableOutputDescriptor::StaticOutput and use the first 8 bytes (I think HD counters are 8 bytes?) of the channel_keys_id as the key derivation index.

[TheBlueMatt]

When we do this we should also make the payment_key in the channel less reliant on as much entropy, so that it can theoretically be derived from the seed if we have to (maybe 32-bits of entropy or something smaller).

[TheBlueMatt]

Gonna mark this 117 cause we at least should do the above sooner rather than later.

[TheBlueMatt]

Unlikely we can get to this in time for 0.1 and there's no use delaying the release for this.