WalletWasabi / WalletWasabi

Open-source, non-custodial, privacy preserving Bitcoin wallet for Windows, Linux, and Mac.
https://wasabiwallet.io
MIT License
2.16k stars 501 forks source link

Why WitScript?.ToString() ? #209

Closed nopara73 closed 6 years ago

nopara73 commented 6 years ago

Right now I am checking everywhere if witscript exists like this:

string.IsNullOrWhiteSpace(transaction.Inputs[index].WitScript?.ToString())

What I want to check is: transaction.Inputs[index].WitScript == null, but I think there was some bug in NBitcoin, that's why I was checking like this in the previous version of HiddenWallet. I didn't wanted to mess with working code, so I kept tostringing it. We should get to the bottom of this issue.

lontivero commented 6 years ago

@varsnotwars you must compare the WitScript against the Script.Empty instance. Take a look at this fragment extracted from the NBitcoin's GolombRiceFilterBuilder class:

if( txin.WitScript != WitScript.Empty)
{
    // Each item within the witness stack of each input (if the input has a witness)
    builder.AddWitness(txin.WitScript);
}

https://github.com/MetacoSA/NBitcoin/blob/4eed3ee57b0194225bb821d961026f038a57a7b1/NBitcoin/BIP158/GolombRiceFilter.cs#L349

lontivero commented 6 years ago

Probably it would be a good idea to have an extension method for this. Something like:

public static bool HasWitness(this TxIn txin)
{
    if(txin == null) throw new ArgumentNullException(nameof(txin));
    return txin.WitScript != null && txin.WitScript != Script.Empty;
}
nopara73 commented 6 years ago

Mystery solved! concept ACK extension, just use the Guard.NotNull for guarding.

nopara73 commented 6 years ago

Btw it's WitScript.Empty.

nopara73 commented 6 years ago

07976c050cb3ca89a88b65500e13e328d9dace17