New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update PSBT test #627
Closed
joemphilips
wants to merge
10
commits into
MetacoSA:master
from
joemphilips:psbt.walletcreatefundedpsbt
Closed
Update PSBT test #627
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
e5d6184
Add Test for WalletCreateFundedPSBT
joemphilips b72434f
add getaddressinfo RPC call
joemphilips 27e6055
Slight modification to PSBT
joemphilips 4d25134
Format RPCClient.Wallet.cs
joemphilips 2b3d899
WIP: write WalletCreateFundedPSBTResponse RPC
joemphilips 7594be8
Update PSBT RPC test and fix bug in finalization
joemphilips e2588fb
Update PSBT
joemphilips 8b84caf
Improve PSBT Finalization.
joemphilips e3b93af
throw AggreateException from PSBT.Finalize
joemphilips cd86479
Test that adding ScriptCoin will always be better than adding scripts…
joemphilips File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,12 +32,14 @@ public class RPCClientTests | |
const string TestAccount = "NBitcoin.RPCClientTests"; | ||
|
||
public PSBTComparer PSBTComparerInstance { get; } | ||
public ITestOutputHelper Output { get; } | ||
|
||
public RPCClientTests() | ||
public RPCClientTests(ITestOutputHelper output) | ||
{ | ||
Arb.Register<PSBTGenerator>(); | ||
Arb.Register<SegwitTransactionGenerators>(); | ||
PSBTComparerInstance = new PSBTComparer(); | ||
Output = output; | ||
} | ||
|
||
[Fact] | ||
|
@@ -1404,8 +1406,8 @@ public void ShouldWalletProcessPSBTAndExtractMempoolAcceptableTX() | |
result = client.WalletProcessPSBT(psbtUnFinalized, false, type, bip32derivs: true); | ||
Assert.False(result.Complete); | ||
Assert.False(result.PSBT.CanExtractTX()); | ||
result.PSBT.TryFinalize(out bool isFinalized2); | ||
Assert.False(isFinalized2); | ||
result.PSBT.TryFinalize(out var errors2); | ||
Assert.NotEmpty(errors2); | ||
foreach (var psbtin in result.PSBT.inputs) | ||
{ | ||
Assert.Equal(SigHash.Undefined, psbtin.SighashType); | ||
|
@@ -1414,10 +1416,161 @@ public void ShouldWalletProcessPSBTAndExtractMempoolAcceptableTX() | |
|
||
// signed | ||
result = client.WalletProcessPSBT(psbtUnFinalized, true, type); | ||
result.PSBT.TryFinalize(out bool isFinalized3); | ||
Assert.True(isFinalized3); | ||
result.PSBT.TryFinalize(out var errors3); | ||
Assert.Empty(errors3); | ||
var txResult = result.PSBT.ExtractTX(); | ||
client.TestMempoolAccept(txResult, true); | ||
var acceptResult = client.TestMempoolAccept(txResult, true); | ||
Assert.True(acceptResult.IsAllowed, acceptResult.RejectReason); | ||
} | ||
} | ||
|
||
// refs: https://github.com/bitcoin/bitcoin/blob/df73c23f5fac031cc9b2ec06a74275db5ea322e3/doc/psbt.md#workflows | ||
// with 2 difference. | ||
// 1. one user (David) do not use bitcoin core (only NBitcoin) | ||
// 2. 4-of-4 instead of 2-of-3 | ||
// 3. In version 0.17, `importmulti` can not handle witness script so only p2sh are considered here. TODO: fix | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not fixed in 0.17.1 ? :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tried but its not fixed :( |
||
[Fact] | ||
public void ShouldPerformMultisigProcessingWithCore() | ||
{ | ||
using (var builder = NodeBuilderEx.Create()) | ||
{ | ||
var nodeAlice = builder.CreateNode(); | ||
var nodeBob = builder.CreateNode(); | ||
var nodeCarol = builder.CreateNode(); | ||
var nodeFunder = builder.CreateNode(); | ||
var david = new Key(); | ||
builder.StartAll(); | ||
|
||
// prepare multisig script and watch with node. | ||
var nodes = new CoreNode[]{nodeAlice, nodeBob, nodeCarol}; | ||
var clients = nodes.Select(n => n.CreateRPCClient()).ToArray(); | ||
var addresses = clients.Select(c => c.GetNewAddress()); | ||
var addrInfos = addresses.Select((a, i) => clients[i].GetAddressInfo(a)); | ||
var pubkeys = new List<PubKey> { david.PubKey }; | ||
pubkeys.AddRange(addrInfos.Select(i => i.PubKey).ToArray()); | ||
var script = PayToMultiSigTemplate.Instance.GenerateScriptPubKey(4, pubkeys.ToArray()); | ||
var aMultiP2SH = script.Hash.ScriptPubKey; | ||
// var aMultiP2WSH = script.WitHash.ScriptPubKey; | ||
// var aMultiP2SH_P2WSH = script.WitHash.ScriptPubKey.Hash.ScriptPubKey; | ||
var multiAddresses = new BitcoinAddress[] { aMultiP2SH.GetDestinationAddress(builder.Network) }; | ||
var importMultiObject = new ImportMultiAddress[] { | ||
new ImportMultiAddress() | ||
{ | ||
ScriptPubKey = new ImportMultiAddress.ScriptPubKeyObject(multiAddresses[0]), | ||
RedeemScript = script.ToHex(), | ||
Internal = true, | ||
}, | ||
/* | ||
new ImportMultiAddress() | ||
{ | ||
ScriptPubKey = new ImportMultiAddress.ScriptPubKeyObject(aMultiP2WSH), | ||
RedeemScript = script.ToHex(), | ||
Internal = true, | ||
}, | ||
new ImportMultiAddress() | ||
{ | ||
ScriptPubKey = new ImportMultiAddress.ScriptPubKeyObject(aMultiP2SH_P2WSH), | ||
RedeemScript = script.WitHash.ScriptPubKey.ToHex(), | ||
Internal = true, | ||
}, | ||
new ImportMultiAddress() | ||
{ | ||
ScriptPubKey = new ImportMultiAddress.ScriptPubKeyObject(aMultiP2SH_P2WSH), | ||
RedeemScript = script.ToHex(), | ||
Internal = true, | ||
} | ||
*/ | ||
}; | ||
|
||
for (var i = 0; i < clients.Length; i++) | ||
{ | ||
var c = clients[i]; | ||
Output.WriteLine($"Importing for {i}"); | ||
c.ImportMulti(importMultiObject, false); | ||
} | ||
|
||
// pay from funder | ||
nodeFunder.Generate(103); | ||
var funderClient = nodeFunder.CreateRPCClient(); | ||
funderClient.SendToAddress(aMultiP2SH, Money.Coins(40)); | ||
// funderClient.SendToAddress(aMultiP2WSH, Money.Coins(40)); | ||
// funderClient.SendToAddress(aMultiP2SH_P2WSH, Money.Coins(40)); | ||
nodeFunder.Generate(1); | ||
foreach (var n in nodes) | ||
{ | ||
nodeFunder.Sync(n, true); | ||
} | ||
|
||
// pay from multisig address | ||
// first carol creates psbt | ||
var carol = clients[2]; | ||
// check if we have enough balance | ||
var info = carol.GetBlockchainInfoAsync().Result; | ||
Assert.Equal((ulong)104, info.Blocks); | ||
var balance = carol.GetBalance(0, true); | ||
// Assert.Equal(Money.Coins(120), balance); | ||
Assert.Equal(Money.Coins(40), balance); | ||
|
||
var aSend = new Key().PubKey.GetAddress(nodeAlice.Network); | ||
var outputs = new Dictionary<BitcoinAddress, Money>(); | ||
outputs.Add(aSend, Money.Coins(10)); | ||
var fundOptions = new FundRawTransactionOptions() { SubtractFeeFromOutputs = new int[] {0}, IncludeWatching = true }; | ||
PSBT psbt = carol.WalletCreateFundedPSBT(null, outputs, 0, fundOptions).PSBT; | ||
psbt = carol.WalletProcessPSBT(psbt).PSBT; | ||
|
||
// second, Bob checks and process psbt. | ||
var bob = clients[1]; | ||
Assert.Contains(multiAddresses, a => | ||
psbt.inputs.Any(psbtin => psbtin.WitnessUtxo?.ScriptPubKey == a.ScriptPubKey) || | ||
psbt.inputs.Any(psbtin => (bool)psbtin.NonWitnessUtxo?.Outputs.Any(o => a.ScriptPubKey == o.ScriptPubKey)) | ||
); | ||
var psbt1 = bob.WalletProcessPSBT(psbt.Clone()).PSBT; | ||
|
||
// at the same time, David may do the ; | ||
psbt.TrySignAll(david); | ||
var alice = clients[0]; | ||
var psbt2 = alice.WalletProcessPSBT(psbt).PSBT; | ||
psbt2.TryFinalize(out var errors); | ||
Assert.NotEmpty(errors); // not enough signature. | ||
|
||
// So let's combine. | ||
var psbtCombined = psbt1.Combine(psbt2); | ||
|
||
// Finally, anyone can finalize and broadcast the psbt. | ||
var tx = psbtCombined.Finalize().ExtractTX(); | ||
var result = alice.TestMempoolAccept(tx); | ||
Assert.True(result.IsAllowed, result.RejectReason); | ||
} | ||
} | ||
|
||
|
||
[Fact] | ||
/// <summary> | ||
/// For p2sh, p2wsh, p2sh-p2wsh, we must also test the case for `solvable` to the wallet. | ||
/// For that, both script and the address must be imported by `importmulti`. | ||
/// but importmulti can not handle witness script(in v0.17). | ||
/// TODO: add test for solvable scripts. | ||
/// </summary> | ||
public void ShouldGetAddressInfo() | ||
{ | ||
using (var builder = NodeBuilderEx.Create()) | ||
{ | ||
var client = builder.CreateNode(true).CreateRPCClient(); | ||
var addrLegacy = client.GetNewAddress(new GetNewAddressRequest() { AddressType = AddressType.Legacy }); | ||
var addrBech32 = client.GetNewAddress(new GetNewAddressRequest() { AddressType = AddressType.Bech32 }); | ||
var addrP2SHSegwit = client.GetNewAddress(new GetNewAddressRequest() { AddressType = AddressType.P2SHSegwit }); | ||
var pubkeys = new PubKey[] { new Key().PubKey, new Key().PubKey, new Key().PubKey }; | ||
var redeem = PayToMultiSigTemplate.Instance.GenerateScriptPubKey(2, pubkeys); | ||
client.ImportAddress(redeem.Hash); | ||
client.ImportAddress(redeem.WitHash); | ||
client.ImportAddress(redeem.WitHash.ScriptPubKey.Hash); | ||
|
||
Assert.NotNull(client.GetAddressInfo(addrLegacy)); | ||
Assert.NotNull(client.GetAddressInfo(addrBech32)); | ||
Assert.NotNull(client.GetAddressInfo(addrP2SHSegwit)); | ||
Assert.NotNull(client.GetAddressInfo(redeem.Hash)); | ||
Assert.NotNull(client.GetAddressInfo(redeem.WitHash)); | ||
Assert.NotNull(client.GetAddressInfo(redeem.WitHash.ScriptPubKey.Hash)); | ||
} | ||
} | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should still
Assert.False
this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed the
TryFinalize
API to takeout InvalidOperationException[]
instead ofout bool
so that the user can see why (and which input) is failing to finalize (breaking change). SoAssert.NotEmpty
below is equal to previousAssert.False
Sorry that I have put too many changes in one PR, I will split.