Tweak a few return types to make it easier to gen C wrappers

Our (naive) C wrapper generator gets easily confused, so we tweak
our source in a few ways to make it easier to understand, namely:
 * Traits must be defined before they are referenced
 * For generic traits, we understand associated types, but not
   direct <Type: TraitBound> parameters
 * Non-C-bindable types in traits will fail to generate wrappers -
   we mark them (C-not exported) for now but will have to get them
   all to work to make the C bindings usable,
 * For constructors, we have to return "Self", not the
   fully-specified (generic) struct name.
 * When specifying generics, we must use only one token (ie
   ChannelMessageHandler, not msgs::ChannelMessageHandler)
 * We can't have `new()` constructors for completely-transparent
   structs (nor should we, really)
 * We cannot use foo::bar::self

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -95,7 +95,9 @@ impl TestChannelMonitor {
 		}
 	}
 }
-impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMonitor {
+impl channelmonitor::ManyChannelMonitor for TestChannelMonitor {
+	type Keys = EnforcingChannelKeys;
+
 	fn add_monitor(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<EnforcingChannelKeys>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
 		let mut ser = VecWriter(Vec::new());
 		monitor.write_for_disk(&mut ser).unwrap();
@@ -194,7 +196,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 			config.channel_options.fee_proportional_millionths = 0;
 			config.channel_options.announced_channel = true;
 			config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
-			(Arc::new(ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0).unwrap()),
+			(Arc::new(ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0)),
 			monitor)
 		} }
 	}
@@ -266,7 +268,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 					let tx = Transaction { version: $chan_id, lock_time: 0, input: Vec::new(), output: vec![TxOut {
 						value: *channel_value_satoshis, script_pubkey: output_script.clone(),
 					}]};
-					funding_output = OutPoint::new(tx.txid(), 0);
+					funding_output = OutPoint { txid: tx.txid(), index: 0 };
 					$source.funding_transaction_generated(&temporary_channel_id, funding_output);
 					channel_txn.push(tx);
 				} else { panic!("Wrong event type"); }

fuzz/src/full_stack.rs

@@ -331,7 +331,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 	config.channel_options.fee_proportional_millionths =  slice_to_be32(get_slice!(4));
 	config.channel_options.announced_channel = get_slice!(1)[0] != 0;
 	config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
-	let channelmanager = Arc::new(ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0).unwrap());
+	let channelmanager = Arc::new(ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0));
 	let router = Arc::new(Router::new(PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret()), watch.clone(), Arc::clone(&logger)));
 
 	let peers = RefCell::new([false; 256]);
@@ -474,7 +474,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 					let funding_output = 'search_loop: loop {
 						let funding_txid = tx.txid();
 						if let None = loss_detector.txids_confirmed.get(&funding_txid) {
-							let outpoint = OutPoint::new(funding_txid, 0);
+							let outpoint = OutPoint { txid: funding_txid, index: 0 };
 							for chan in channelmanager.list_channels() {
 								if chan.channel_id == outpoint.to_channel_id() {
 									tx.version += 1;

lightning/src/chain/chaininterface.rs

@@ -38,6 +38,7 @@ pub enum ChainError {
 /// Note that all of the functions implemented here *must* be reentrant-safe (obviously - they're
 /// called from inside the library in response to ChainListener events, P2P events, or timer
 /// events).
+/// (C-not exported) due to tuples and Result<>s
 pub trait ChainWatchInterface: Sync + Send {
 	/// Provides a txid/random-scriptPubKey-in-the-tx which much be watched for.
 	fn install_watch_tx(&self, txid: &Txid, script_pub_key: &Script);
@@ -72,6 +73,7 @@ pub trait BroadcasterInterface: Sync + Send {
 }
 
 /// A trait indicating a desire to listen for events from the chain
+/// (C-not exported)
 pub trait ChainListener: Sync + Send {
 	/// Notifies a listener that a block was connected.
 	///
@@ -128,6 +130,7 @@ pub trait FeeEstimator: Sync + Send {
 pub const MIN_RELAY_FEE_SAT_PER_1000_WEIGHT: u64 = 4000;
 
 /// Utility for tracking registered txn/outpoints and checking for matches
+/// (C-not exported)
 #[cfg_attr(test, derive(PartialEq))]
 pub struct ChainWatchedUtil {
 	watch_all: bool,
@@ -169,6 +172,7 @@ impl ChainWatchedUtil {
 
 	/// Registers an outpoint for monitoring, returning true if it was a new outpoint and false if
 	/// we'd already been watching for it
+	/// (C-not exported) due to tuples
 	pub fn register_outpoint(&mut self, outpoint: (Txid, u32), _script_pub_key: &Script) -> bool {
 		if self.watch_all { return false; }
 		self.watched_outpoints.insert(outpoint)
@@ -220,6 +224,7 @@ impl ChainWatchedUtil {
 /// parameters with static lifetimes). Other times you can afford a reference, which is more
 /// efficient, in which case BlockNotifierRef is a more appropriate type. Defining these type
 /// aliases prevents issues such as overly long function definitions.
+/// (C-not exported)
 pub type BlockNotifierArc = Arc<BlockNotifier<'static, Arc<ChainListener>>>;
 
 /// BlockNotifierRef is useful when you want a BlockNotifier that points to ChainListeners
@@ -238,6 +243,7 @@ pub type BlockNotifierRef<'a> = BlockNotifier<'a, &'a ChainListener>;
 /// or a BlockNotifierRef for conciseness. See their documentation for more details, but essentially
 /// you should default to using a BlockNotifierRef, and use a BlockNotifierArc instead when you
 /// require ChainListeners with static lifetimes, such as when you're using lightning-net-tokio.
+/// (C-not exported) due to ChainListener not being exported
 pub struct BlockNotifier<'a, CL: Deref<Target = ChainListener + 'a> + 'a> {
 	listeners: Mutex<Vec<CL>>,
 	chain_monitor: Arc<ChainWatchInterface>,
@@ -312,6 +318,7 @@ impl<'a, CL: Deref<Target = ChainListener + 'a> + 'a> BlockNotifier<'a, CL> {
 /// Utility to capture some common parts of ChainWatchInterface implementors.
 ///
 /// Keeping a local copy of this in a ChainWatchInterface implementor is likely useful.
+/// (C-not exported)
 pub struct ChainWatchInterfaceUtil {
 	network: Network,
 	watched: Mutex<ChainWatchedUtil>,

lightning/src/chain/keysinterface.rs

@@ -142,28 +142,6 @@ impl Readable for SpendableOutputDescriptor {
 	}
 }
 
-/// A trait to describe an object which can get user secrets and key material.
-pub trait KeysInterface: Send + Sync {
-	/// A type which implements ChannelKeys which will be returned by get_channel_keys.
-	type ChanKeySigner : ChannelKeys;
-
-	/// Get node secret key (aka node_id or network_key)
-	fn get_node_secret(&self) -> SecretKey;
-	/// Get destination redeemScript to encumber static protocol exit points.
-	fn get_destination_script(&self) -> Script;
-	/// Get shutdown_pubkey to use as PublicKey at channel closure
-	fn get_shutdown_pubkey(&self) -> PublicKey;
-	/// Get a new set of ChannelKeys for per-channel secrets. These MUST be unique even if you
-	/// restarted with some stale data!
-	fn get_channel_keys(&self, inbound: bool, channel_value_satoshis: u64) -> Self::ChanKeySigner;
-	/// Get a secret and PRNG seed for construting an onion packet
-	fn get_onion_rand(&self) -> (SecretKey, [u8; 32]);
-	/// Get a unique temporary channel id. Channels will be referred to by this until the funding
-	/// transaction is created, at which point they will use the outpoint in the funding
-	/// transaction.
-	fn get_channel_id(&self) -> [u8; 32];
-}
-
 /// Set of lightning keys needed to operate a channel as described in BOLT 3.
 ///
 /// Signing services could be implemented on a hardware wallet. In this case,
@@ -192,26 +170,34 @@ pub trait KeysInterface: Send + Sync {
 // TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
 // ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
 pub trait ChannelKeys : Send+Clone {
+	/// (C-not exported) due to references
 	/// Gets the private key for the anchor tx
 	fn funding_key<'a>(&'a self) -> &'a SecretKey;
 	/// Gets the local secret key for blinded revocation pubkey
+	/// (C-not exported) due to references
 	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
 	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
 	/// output to us in transactions our counterparty broadcasts).
 	/// Also as part of obscured commitment number.
+	/// (C-not exported) due to references
 	fn payment_key<'a>(&'a self) -> &'a SecretKey;
 	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
+	/// (C-not exported) due to references
 	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
 	/// Gets the local htlc secret key used in commitment tx htlc outputs
+	/// (C-not exported) due to references
 	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
 	/// Gets the commitment seed
+	/// (C-not exported) due to references
 	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
 	/// Gets the local channel public keys and basepoints
+	/// (C-not exported) due to references
 	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys;
 
 	/// Create a signature for a remote commitment transaction and associated HTLC transactions.
 	///
 	/// Note that if signing fails or is rejected, the channel will be force-closed.
+	/// (C-not exported) due to references
 	//
 	// TODO: Document the things someone using this interface should enforce before signing.
 	// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
@@ -221,6 +207,7 @@ pub trait ChannelKeys : Send+Clone {
 	/// Create a signature for a local commitment transaction. This will only ever be called with
 	/// the same local_commitment_tx (or a copy thereof), though there are currently no guarantees
 	/// that it will not be called multiple times.
+	/// (C-not exported) due to references
 	//
 	// TODO: Document the things someone using this interface should enforce before signing.
 	// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
@@ -246,12 +233,14 @@ pub trait ChannelKeys : Send+Clone {
 	/// (implying they were considered dust at the time the commitment transaction was negotiated),
 	/// a corresponding None should be included in the return value. All other positions in the
 	/// return value must contain a signature.
+	/// (C-not exported) due to references
 	fn sign_local_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, local_commitment_tx: &LocalCommitmentTransaction, local_csv: u16, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
 
 	/// Create a signature for a (proposed) closing transaction.
 	///
 	/// Note that, due to rounding, there may be one "missing" satoshi, and either party may have
 	/// chosen to forgo their output as dust.
+	/// (C-not exported) due to references
 	fn sign_closing_transaction<T: secp256k1::Signing>(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
 	/// Signs a channel announcement message with our funding key, proving it comes from one
@@ -260,15 +249,42 @@ pub trait ChannelKeys : Send+Clone {
 	/// Note that if this fails or is rejected, the channel will not be publicly announced and
 	/// our counterparty may (though likely will not) close the channel on us for violating the
 	/// protocol.
+	/// (C-not exported) due to references
 	fn sign_channel_announcement<T: secp256k1::Signing>(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
 	/// Set the remote channel basepoints.  This is done immediately on incoming channels
 	/// and as soon as the channel is accepted on outgoing channels.
 	///
 	/// Will be called before any signatures are applied.
+	/// (C-not exported) due to references
 	fn set_remote_channel_pubkeys(&mut self, channel_points: &ChannelPublicKeys);
 }
 
+
+/// A trait to describe an object which can get user secrets and key material.
+pub trait KeysInterface: Send + Sync {
+	/// A type which implements ChannelKeys which will be returned by get_channel_keys.
+	type ChanKeySigner : ChannelKeys;
+
+	/// Get node secret key (aka node_id or network_key)
+	fn get_node_secret(&self) -> SecretKey;
+	/// Get destination redeemScript to encumber static protocol exit points.
+	fn get_destination_script(&self) -> Script;
+	/// Get shutdown_pubkey to use as PublicKey at channel closure
+	fn get_shutdown_pubkey(&self) -> PublicKey;
+	/// Get a new set of ChannelKeys for per-channel secrets. These MUST be unique even if you
+	/// restarted with some stale data!
+	/// (C-not exported) due to Self (though it *should* work...)
+	fn get_channel_keys(&self, inbound: bool, channel_value_satoshis: u64) -> Self::ChanKeySigner;
+	/// Get a secret and PRNG seed for construting an onion packet
+	/// (C-not exported) due to tuple
+	fn get_onion_rand(&self) -> (SecretKey, [u8; 32]);
+	/// Get a unique temporary channel id. Channels will be referred to by this until the funding
+	/// transaction is created, at which point they will use the outpoint in the funding
+	/// transaction.
+	fn get_channel_id(&self) -> [u8; 32];
+}
+
 #[derive(Clone)]
 /// A simple implementation of ChannelKeys that just keeps the private keys in memory.
 pub struct InMemoryChannelKeys {
@@ -294,6 +310,7 @@ pub struct InMemoryChannelKeys {
 
 impl InMemoryChannelKeys {
 	/// Create a new InMemoryChannelKeys
+	/// (C-not exported) due to secp context
 	pub fn new<C: Signing>(
 		secp_ctx: &Secp256k1<C>,
 		funding_key: SecretKey,
@@ -509,7 +526,7 @@ impl KeysManager {
 	/// Note that until the 0.1 release there is no guarantee of backward compatibility between
 	/// versions. Once the library is more fully supported, the docs will be updated to include a
 	/// detailed description of the guarantee.
-	pub fn new(seed: &[u8; 32], network: Network, logger: Arc<Logger>, starting_time_secs: u64, starting_time_nanos: u32) -> KeysManager {
+	pub fn new(seed: &[u8; 32], network: Network, logger: Arc<Logger>, starting_time_secs: u64, starting_time_nanos: u32) -> Self {
 		let secp_ctx = Secp256k1::signing_only();
 		match ExtendedPrivKey::new_master(network.clone(), seed) {
 			Ok(master_key) => {

lightning/src/chain/transaction.rs

@@ -16,11 +16,6 @@ pub struct OutPoint {
 }
 
 impl OutPoint {
-	/// Creates a new `OutPoint` from the txid and the index.
-	pub fn new(txid: Txid, index: u16) -> OutPoint {
-		OutPoint { txid, index }
-	}
-
 	/// Convert an `OutPoint` to a lightning channel id.
 	pub fn to_channel_id(&self) -> [u8; 32] {
 		let mut res = [0; 32];
@@ -31,6 +26,7 @@ impl OutPoint {
 	}
 
 	/// Converts this OutPoint into the OutPoint field as used by rust-bitcoin
+	/// (C-not exported)
 	pub fn into_bitcoin_outpoint(self) -> BitcoinOutPoint {
 		BitcoinOutPoint {
 			txid: self.txid,

lightning/src/ln/chan_utils.rs

@@ -5,7 +5,8 @@
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::opcodes;
 use bitcoin::blockdata::transaction::{TxIn,TxOut,OutPoint,Transaction, SigHashType};
-use bitcoin::consensus::encode::{self, Decodable, Encodable};
+use bitcoin::consensus::encode::{Decodable, Encodable};
+use bitcoin::consensus::encode;
 use bitcoin::util::bip143;
 
 use bitcoin::hashes::{Hash, HashEngine};
@@ -480,6 +481,7 @@ pub fn build_htlc_transaction(prev_hash: &Txid, feerate_per_kw: u64, to_self_del
 /// We use this to track local commitment transactions and put off signing them until we are ready
 /// to broadcast. Eventually this will require a signer which is possibly external, but for now we
 /// just pass in the SecretKeys required.
+/// (C-not exported)
 pub struct LocalCommitmentTransaction {
 	// TODO: We should migrate away from providing the transaction, instead providing enough to
 	// allow the ChannelKeys to construct it from scratch. Luckily we already have HTLC data here,

lightning/src/ln/channel.rs

@@ -1493,7 +1493,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
 		}
 
-		let funding_txo = OutPoint::new(msg.funding_txid, msg.funding_output_index);
+		let funding_txo = OutPoint{ txid: msg.funding_txid, index: msg.funding_output_index };
 		self.funding_txo = Some(funding_txo.clone());
 
 		let (remote_initial_commitment_tx, local_initial_commitment_tx, our_signature) = match self.funding_created_signature(&msg.signature) {
@@ -4395,7 +4395,7 @@ mod tests {
 		let tx = Transaction { version: 1, lock_time: 0, input: Vec::new(), output: vec![TxOut {
 			value: 10000000, script_pubkey: output_script.clone(),
 		}]};
-		let funding_outpoint = OutPoint::new(tx.txid(), 0);
+		let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
 		let funding_created_msg = node_a_chan.get_outbound_funding_created(funding_outpoint).unwrap();
 		let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg).unwrap();
 
@@ -4460,7 +4460,7 @@ mod tests {
 		chan.their_to_self_delay = 144;
 		chan.our_dust_limit_satoshis = 546;
 
-		let funding_info = OutPoint::new(Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), 0);
+		let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 };
 		chan.funding_txo = Some(funding_info);
 
 		let their_pubkeys = ChannelPublicKeys {