MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.87k stars 846 forks source link

Allow setting sequence numbers of inputs with TransactionBuilder #974

Closed canndrew closed 3 years ago

canndrew commented 3 years ago

This PR adds a new CoinOptions type defined as:

public class CoinOptions
{
    public CoinOptions()
    {
        Sequence = null;
    }

    public Sequence? Sequence;
}

As well a new AddCoin method to TransactionBuilder which takes an additional CoinOptions argument. This allows setting the sequence number on transaction inputs.

Fixes #973

knocte commented 3 years ago

@NicolasDorier any clue about why the CI timed out here?

NicolasDorier commented 3 years ago

No idea, but probably a bug in this PR. This also need tests

canndrew commented 3 years ago

Tests are passing for this now, except for one of the pipelines though that appears to be a random failure unrelated to the PR.

knocte commented 3 years ago

The test failure seems indeed unrelated:

[xUnit.net 00:03:42.20]         /home/runner/work/NBitcoin/NBitcoin/NBitcoin.Tests/ProtocolTests.cs(702,0): at NBitcoin.Tests.ProtocolTests.CanConnectToRandomNode()
  X NBitcoin.Tests.ProtocolTests.CanConnectToRandomNode [27s 138ms]
  Error Message:
   System.InvalidOperationException : The node is not in a connected state
  Stack Trace:
     at NBitcoin.Protocol.NodeListener.ReceivePayload[TPayload](CancellationToken cancellationToken) in /home/runner/work/NBitcoin/NBitcoin/NBitcoin/Protocol/NodeListener.cs:line 69
   at NBitcoin.Protocol.Node.VersionHandshake(NodeRequirement requirements, CancellationToken cancellationToken) in /home/runner/work/NBitcoin/NBitcoin/NBitcoin/Protocol/Node.cs:line 1045
   at NBitcoin.Protocol.Node.VersionHandshake(CancellationToken cancellationToken) in /home/runner/work/NBitcoin/NBitcoin/NBitcoin/Protocol/Node.cs:line 1034
   at NBitcoin.Tests.ProtocolTests.CanConnectToRandomNode() in /home/runner/work/NBitcoin/NBitcoin/NBitcoin.Tests/ProtocolTests.cs:line 702

Maybe this test should be retried several times in the test suite? So long as it can connect once successfully, it should pass.

@NicolasDorier can you review/merge?

NicolasDorier commented 3 years ago

Awesome, merging. I will change 2 or 3 things about it but that's great.

NicolasDorier commented 3 years ago

I released 5.0.74