From 77ee22792aaf98585a10ceb6272cdb8329c04487 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 20 Oct 2020 15:16:33 +0200 Subject: [PATCH 1/2] Nit: replace 0xFFFFFFFF with a constant, for readability and SSoT --- NBitcoin/Sequence.cs | 10 ++++++---- NBitcoin/Transaction.cs | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/NBitcoin/Sequence.cs b/NBitcoin/Sequence.cs index 48604b3344..ca7c400e50 100644 --- a/NBitcoin/Sequence.cs +++ b/NBitcoin/Sequence.cs @@ -62,7 +62,7 @@ public static Sequence FeeSnipping /// relative lock-time. Setting the most significant bit of a /// sequence number disabled relative lock-time. /// - public const uint SEQUENCE_FINAL = 0xffffffff; + public const uint SEQUENCE_FINAL = UINT_MAX_VALUE; /// /// Setting nSequence to this value on any input in a transaction @@ -82,17 +82,19 @@ public static Sequence FeeSnipping internal const int SEQUENCE_LOCKTIME_GRANULARITY = 9; + internal const uint UINT_MAX_VALUE = 0xFFFFFFFF; + uint _ValueInv; public uint Value { get { - return 0xFFFFFFFF - _ValueInv; + return UINT_MAX_VALUE - _ValueInv; } } public Sequence(uint value) { - _ValueInv = 0xFFFFFFFF - value; + _ValueInv = UINT_MAX_VALUE - value; } public Sequence(int lockHeight) @@ -107,7 +109,7 @@ public Sequence(TimeSpan period) throw new ArgumentOutOfRangeException("Relative lock time must be positive and lower or equals to " + (0xFFFF * 512) + " seconds (approx 388 days)"); var value = (uint)(period.TotalSeconds / (1 << Sequence.SEQUENCE_LOCKTIME_GRANULARITY)); value |= SEQUENCE_LOCKTIME_TYPE_FLAG; - _ValueInv = 0xFFFFFFFF - (uint)value; + _ValueInv = UINT_MAX_VALUE - (uint)value; } public bool IsRelativeLock diff --git a/NBitcoin/Transaction.cs b/NBitcoin/Transaction.cs index 1c659d3dad..e167af17d3 100644 --- a/NBitcoin/Transaction.cs +++ b/NBitcoin/Transaction.cs @@ -1286,7 +1286,7 @@ public bool RBF { get { - return Inputs.Any(i => i.Sequence < 0xffffffff - 1); + return Inputs.Any(i => i.Sequence < Sequence.UINT_MAX_VALUE - 1); } } From f8ed69baeb2073ec36dc8e0fba8d6ed9848daf14 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 20 Oct 2020 16:20:37 +0200 Subject: [PATCH 2/2] TransactionBuilder: stop having RBF disabled by default Old CreateTransactionBuilder() methods are now marked as Obsolete; the new ones, introduced in this commit, will force the NBitcoin consumer to make a decision about what kind of transactions will be created by her/his TransactionBuilder instance: * if using EnhanceOnChainPrivacy mode, RBF will be disabled and the inputs & outputs will be shuffled; * if using Lightning mode, RBF will be enabled (ReplaceByFee is very important in this environment because fee rates might change a lot since the moment when a channel was opened until when it needs to be closed), and shuffling inputs or outputs will be disabled. Why disabling shuffling for Lightning? Because according to the spec, they need to be ordered lexicographically (e.g. the previous defaults were causing a bug in DotNetLightning: [1]). [1] https://github.com/joemphilips/DotNetLightning/pull/133 --- NBitcoin/ConsensusFactory.cs | 8 ++++---- NBitcoin/Network.cs | 12 +++++++++--- NBitcoin/Transaction.cs | 1 + NBitcoin/TransactionBuilder.cs | 20 ++++++++++++++++++-- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/NBitcoin/ConsensusFactory.cs b/NBitcoin/ConsensusFactory.cs index 83028772e5..77a2f6d61c 100644 --- a/NBitcoin/ConsensusFactory.cs +++ b/NBitcoin/ConsensusFactory.cs @@ -127,14 +127,14 @@ public virtual TxOut CreateTxOut() return new TxOut(); } - protected virtual TransactionBuilder CreateTransactionBuilderCore(Network network) + protected virtual TransactionBuilder CreateTransactionBuilderCore(Network network, TransactionCreationMode mode) { - return new TransactionBuilder(network); + return new TransactionBuilder(network, mode); } - internal TransactionBuilder CreateTransactionBuilderCore2(Network network) + internal TransactionBuilder CreateTransactionBuilderCore2(Network network, TransactionCreationMode mode) { - return CreateTransactionBuilderCore(network); + return CreateTransactionBuilderCore(network, mode); } } } diff --git a/NBitcoin/Network.cs b/NBitcoin/Network.cs index 840cfb7f84..c6c7095dde 100644 --- a/NBitcoin/Network.cs +++ b/NBitcoin/Network.cs @@ -2564,19 +2564,25 @@ internal NetworkStringParser NetworkStringParser set; } = new NetworkStringParser(); + [Obsolete("Use CreateTransactionBuilder(TransactionCreationMode)")] public TransactionBuilder CreateTransactionBuilder() { - var builder = this.Consensus.ConsensusFactory.CreateTransactionBuilderCore2(this); - return builder; + return CreateTransactionBuilder(TransactionCreationMode.EnhanceOnChainPrivacy); } public TransactionBuilder CreateTransactionBuilder(int seed) { - var builder = this.Consensus.ConsensusFactory.CreateTransactionBuilderCore2(this); + var builder = this.Consensus.ConsensusFactory.CreateTransactionBuilderCore2(this, TransactionCreationMode.EnhanceOnChainPrivacy); builder.ShuffleRandom = new Random(seed); return builder; } + public TransactionBuilder CreateTransactionBuilder(TransactionCreationMode mode) + { + var builder = this.Consensus.ConsensusFactory.CreateTransactionBuilderCore2(this, mode); + return builder; + } + public Base58CheckEncoder GetBase58CheckEncoder() { return NetworkStringParser.GetBase58CheckEncoder(); diff --git a/NBitcoin/Transaction.cs b/NBitcoin/Transaction.cs index e167af17d3..6d51a0bc6b 100644 --- a/NBitcoin/Transaction.cs +++ b/NBitcoin/Transaction.cs @@ -1280,6 +1280,7 @@ internal void ReadWrite(BitcoinStream stream) //https://en.bitcoin.it/wiki/Transactions //https://en.bitcoin.it/wiki/Protocol_specification + [Obsolete("Use Network.CreateTransactionBuilder() for a better API to create transactions")] public class Transaction : IBitcoinSerializable { public bool RBF diff --git a/NBitcoin/TransactionBuilder.cs b/NBitcoin/TransactionBuilder.cs index 65fac9a0b4..6f75b22146 100644 --- a/NBitcoin/TransactionBuilder.cs +++ b/NBitcoin/TransactionBuilder.cs @@ -302,6 +302,18 @@ public IMoney Missing } } + public enum TransactionCreationMode + { + /// + /// RBF disabled, shuffled inputs and shuffled outputs + /// + EnhanceOnChainPrivacy, + /// + /// RBF enabled, deterministic inputs and outputs (not shuffled) + /// + Lightning, + } + /// /// A class for building and signing all sort of transactions easily (http://www.codeproject.com/Articles/835098/NBitcoin-Build-Them-All) /// @@ -706,7 +718,7 @@ internal BuilderGroup CurrentGroup } } - internal TransactionBuilder(Network network) + internal TransactionBuilder(Network network, TransactionCreationMode mode) { if (network == null) throw new ArgumentNullException(nameof(network)); @@ -715,7 +727,11 @@ internal TransactionBuilder(Network network) CoinSelector = new DefaultCoinSelector(ShuffleRandom); StandardTransactionPolicy = new StandardTransactionPolicy(); DustPrevention = true; - OptInRBF = false; + + OptInRBF = (mode == TransactionCreationMode.Lightning); + ShuffleInputs = (mode == TransactionCreationMode.EnhanceOnChainPrivacy); + ShuffleOutputs = ShuffleInputs; + InitExtensions(); }