dgarage / NBXplorer

NBitcoin Explorer
MIT License
320 stars 212 forks source link

Broadcasting BCH/BTG Tx fails with Signature must use SIGHASH_FORKID #52

Closed bazooka70 closed 6 years ago

bazooka70 commented 6 years ago
builder.SendEstimatedFees(feeRate);
var tx = builder.BuildTransaction(true);
Assert.True(tester.Client.Broadcast(tx).Success); // <- FAIL

This fails with:

BCH: Transaction 3d13e1a11893ba832319eef1a0c77d397dc8cc7c8c8e7a22dd3c91552c277615 failed to broadcast (Code: RPC_TRANSACTION_REJECTED, Message: Transaction was rejected by network rules, Details: 16: mandatory-script-verify-flag-failed (Signature must use SIGHASH_FORKID) )

Please, what could cause this, and how can I resolve this?

The same issue is true with BTG network.

Related: https://github.com/dgarage/NBXplorer/pull/50

bazooka70 commented 6 years ago

If I'm reading the docs correctly SigHash should be added another flag FORKID == 64 (0x40) Then var tx = builder.BuildTransaction(true, ALL|FORKID); should work. still checking into it...

NicolasDorier commented 6 years ago

I don't think so, I think the signature process is quite different. I probably need to refactor NBitcoin to allow Network specific signature schemes.

bazooka70 commented 6 years ago

I was quite sure there should be another flag FORKID. https://bitcoin.stackexchange.com/questions/58012/what-is-the-exact-difference-between-a-bitcoin-and-bitcoin-cash-raw-transaction/58030#58030

I added this flag to enum SigHash and tested it via: tx = builder.BuildTransaction(true, sigHash, SigHash.All | SigHash.Forkid);

and obviously TransactionBuilder told me Script error on input 0 (SigHashType). Sorry, but I know very little about this issue. :/

NicolasDorier commented 6 years ago

yeah I probably know why

NicolasDorier commented 6 years ago

trying to work on a simple solution

bazooka70 commented 6 years ago

Did you have any luck with this issue?

NicolasDorier commented 6 years ago

Yes, I started refactoring to make it possible, but still some things to do.

NicolasDorier commented 6 years ago

Closing this one. Basically you need to call TransactionBuilder.SetConsensusFactory(network) and it will sign correctly without you having to change your code.

Will update the references soon.

NicolasDorier commented 6 years ago

Done, have fun.

bazooka70 commented 6 years ago

Wow! thanks! I will run some tests asap.

BTW,

Basically you need to call TransactionBuilder.SetConsensusFactory(network) ...

Shouldn't the TransactionBuilder always call SetConsensusFactory for any network?

bazooka70 commented 6 years ago

I have installed the latest NuGet packages. TransactionBuilder.Verify() returns false and error:

Script error on input 0 (SigHashType)

My code:

            ...
            builder.SendEstimatedFees(feeRate);
            builder.SetConsensusFactory(network);
            Transaction tx = builder.BuildTransaction(true); // build and sign the transaction

            NBitcoin.Policy.TransactionPolicyError[] errors;
            bool builderVerify = builder.Verify(tx, out errors);
            if (!builderVerify)
            {
                Console.WriteLine($"Error builderVerify!");
                foreach (var e in errors)
                {
                    Console.WriteLine("builder error-> " + e.ToString());
                }                
                Console.ReadKey();
                return;
            }
            // broadcast Tx
           ...

I will double check gain...

bazooka70 commented 6 years ago

OK, I double checked. The builder.Verify returns false and an error as described above, but the transaction Broadcast is successful.

NicolasDorier commented 6 years ago

Thanks a lot v4.1.1.3 on its way.

NicolasDorier commented 6 years ago

updated, it works now

bazooka70 commented 6 years ago

Great news! thanks. can you please answer my question: Shouldn't the TransactionBuilder by default, always call SetConsensusFactory() for any network (also for BTC)? Is there any reason why we need to explicitly call SetConsensusFactory?

NicolasDorier commented 6 years ago

Because if I don't know the specific network you are signing, I can't know which algorithm to use to sign properly.

bazooka70 commented 6 years ago

Ok, I had the assumption TransactionBuilder knows about the network at some point of the build...