BTW, we should be happy Satoshi choose 1 as the error code and not 0, because 0 is a weak message for a ECDSA hash and can be easily forged. This would result in anybody being able to steal each other coins.
(or maybe he knew about this fact)
Topic: What is up with this SIGHASH_SINGLE and nOut out of range? (Read 1870 times)
yeah it returns one, but the tx is still valid in the blockchain. i know it's a big wtf.
the actual hash looks like 00 00 00 ... 00 01 (last byte is 0x01 but all the rest is 0x00).
the actual hash looks like 00 00 00 ... 00 01 (last byte is 0x01 but all the rest is 0x00).
Now we've found the first and only way to test the execution of the line:
Code:
// Drop the signature, since there's no way for a signature to sign itself
scriptCode.FindAndDelete(CScript(vchSig));
scriptCode.FindAndDelete(CScript(vchSig));
(for which I thought it would be impossible to ever test in a live transaction)
Since the hash "00 .. 01" does not depend on the script hash that spends the output, you can create a scriptpub that actually includes the signature of the hash "00 .. 01" in the script.
For example:
Will result in
Nevertheless since it won't affect in any way the outcome of the signature checking, you'll only be checking if that execution path aborts with an exception or continues normally.
Oh, no!
Thanks for the advise to make my life easy, but switching the validation off, after I had spent so much effort on validating the txs, is like admitting that I failed on the most important part.
I know that there are still things that will make my code fail, eventually somewhere in a future, but that is exactly where I need people like you, who try to break it now, while I seem to be the only user, thus with no big consequences.
So please keep up the good job, and let me handle whatever you break
Thanks for the advise to make my life easy, but switching the validation off, after I had spent so much effort on validating the txs, is like admitting that I failed on the most important part.
I know that there are still things that will make my code fail, eventually somewhere in a future, but that is exactly where I need people like you, who try to break it now, while I seem to be the only user, thus with no big consequences.
So please keep up the good job, and let me handle whatever you break
Successful people are willing to cut their losses and refocus their efforts. You can always retrieve the code from revision control history later when the testing infrastructure improves; without good testing capabilities you're wasting your time.
Quote
The general who advances without coveting fame and retreats without fearing disgrace, whose only thought is to protect his country and do good service for his sovereign, is the jewel of the kingdom.
yeah it returns one, but the tx is still valid in the blockchain. i know it's a big wtf.
the actual hash looks like 00 00 00 ... 00 01 (last byte is 0x01 but all the rest is 0x00).
the actual hash looks like 00 00 00 ... 00 01 (last byte is 0x01 but all the rest is 0x00).
FWIW I'd suggest you consider removing script evaluation from your library entirely for now. Keep the rest of the scripting code - you need it to create transactions after all - but make it clear that the only way you know that a transaction is valid is if a miner mines it. This is more in-line with the guarantees SPV clients can provide anyway, and will save you an enormous amount of work that is probably wasted until we have better testing infrastructure for third party clients to use.
Oh, no!Thanks for the advise to make my life easy, but switching the validation off, after I had spent so much effort on validating the txs, is like admitting that I failed on the most important part.
I know that there are still things that will make my code fail, eventually somewhere in a future, but that is exactly where I need people like you, who try to break it now, while I seem to be the only user, thus with no big consequences.
So please keep up the good job, and let me handle whatever you break
BTW, it would be nice to a have a test tx that triggers this code:
Code:
// In case concatenating two scripts ends up with two codeseparators,
// or an extra one at the end, this prevents all those possible incompatibilities.
scriptCode.FindAndDelete(CScript(OP_CODESEPARATOR));
// or an extra one at the end, this prevents all those possible incompatibilities.
scriptCode.FindAndDelete(CScript(OP_CODESEPARATOR));
Working on it.
FWIW I'd suggest you consider removing script evaluation from your library entirely for now. Keep the rest of the scripting code - you need it to create transactions after all - but make it clear that the only way you know that a transaction is valid is if a miner mines it. This is more in-line with the guarantees SPV clients can provide anyway, and will save you an enormous amount of work that is probably wasted until we have better testing infrastructure for third party clients to use.
We do that already. This issue with SIGHASH_SINGLE was documented in the unittests here: https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_valid.json#L39 (I did not find it first)
The other main source of test cases is Matt Corallo's block tester: https://github.com/TheBlueMatt/test-scripts
Thanks! The vectors from the json file helped me a lot today. And the tx_invalid.json also turned out to be useful.The other main source of test cases is Matt Corallo's block tester: https://github.com/TheBlueMatt/test-scripts
Who would have thought that cutting off the signature from a message that gets hashed is such a complex operation (talking about the length prefix here).
Though, the Matt's block tester - I did not even try to build it, not too mention figuring out how to use it.. Maybe some day.
You're welcome to go through those test cases and summarize the more subtle ones, but frankly I don't see the point: any change, no matter how small, triggers a hard fork. Perfect compliance with those tests is a necessary but far from sufficient requirement.
Sure - but at least this many less to go 
BTW, it would be nice to a have a test tx that triggers this code:
Code:
// In case concatenating two scripts ends up with two codeseparators,
// or an extra one at the end, this prevents all those possible incompatibilities.
scriptCode.FindAndDelete(CScript(OP_CODESEPARATOR));
// or an extra one at the end, this prevents all those possible incompatibilities.
scriptCode.FindAndDelete(CScript(OP_CODESEPARATOR));
Just as a general comment, to anyone reading this: please always, whenever you discover such a case, do whatever you can to public any possible sources of hard forks that can be a consequence of an existing implementation, that is not an obvious design decision, but rather a hard to spot, implementation specific whatever (a bug or a feature).
We do that already. This issue with SIGHASH_SINGLE was documented in the unittests here: https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_valid.json#L39 (I did not find it first)
The other main source of test cases is Matt Corallo's block tester: https://github.com/TheBlueMatt/test-scripts
You're welcome to go through those test cases and summarize the more subtle ones, but frankly I don't see the point: any change, no matter how small, triggers a hard fork. Perfect compliance with those tests is a necessary but far from sufficient requirement.
Just as a general comment, to anyone reading this: please always, whenever you discover such a case, do whatever you can to public any possible sources of hard forks that can be a consequence of an existing implementation, that is not an obvious design decision, but rather a hard to spot, implementation specific whatever (a bug or a feature).
+1I think such transactions must be treated as non-standard
Just as a general comment, to anyone reading this: please always, whenever you discover such a case, do whatever you can to public any possible sources of hard forks that can be a consequence of an existing implementation, that is not an obvious design decision, but rather a hard to spot, implementation specific whatever (a bug or a feature).
As they say it: knowledge is power.
And knowledge of how you can attack a 1+bn USD worth of a currency is a power worth exploiting.
So if you are honest people, and I think you are, please do not withhold such an important info, but rather advertise it as much as you can - embedding test cases in the block chain is the perfect ultimate solution.
As they say it: knowledge is power.
And knowledge of how you can attack a 1+bn USD worth of a currency is a power worth exploiting.
So if you are honest people, and I think you are, please do not withhold such an important info, but rather advertise it as much as you can - embedding test cases in the block chain is the perfect ultimate solution.
Live with it ok, but we need to know what is the good behavior!
Doesn't this look like a crazy hack/bug?
Thus my question.Doesn't this look like a crazy hack/bug?
For me it seems like someone has overlooked it at some point; "return 1" was supposed to indicate an error, but it was somehow overlooked and now it implicitly gets cast to (uint256)1 which is a pretty valid hash value, so C++ compiler does not complain and so nobody has noticed..
Don't you have the same problem in Armory?
I cannot imagine reproducing the same behavior in python, just by repeating such a mistake.
This was noticed a long time ago. It is just now being re-noticed by others
I discovered this over a year ago while rewriting everything into python:
https://github.com/jgarzik/python-bitcoinlib/
https://github.com/jgarzik/python-bitcoinlib/blob/master/bitcoin/scripteval.py
Can you spot the problem?
Unless your goal is to make sure that the clients you develop keep the monopoly.
Then its probably a good strategy, to not make such an info public.
I believe Peter created these transactions, and I think that was a good idea. Go Peter!
Yes.As much as I don't get along with him in general, thank you @Peter for bringing these txs to the network!
I was always worried about different implementation/language related technicals that could be exploited to create a hard fork, so I was designing my node to be less strict in any checking, for such cases. Since it is not a mining node, it should not matter much, but Peter has actually managed to find a case where my implementation was more strict..
And now I have at least one less to go
Live with it ok, but we need to know what is the good behavior!
Doesn't this look like a crazy hack/bug?
Thus my question.Doesn't this look like a crazy hack/bug?
For me it seems like someone has overlooked it at some point; "return 1" was supposed to indicate an error, but it was somehow overlooked and now it implicitly gets cast to (uint256)1 which is a pretty valid hash value, so C++ compiler does not complain and so nobody has noticed..
Don't you have the same problem in Armory?
I cannot imagine reproducing the same behavior in python, just by repeating such a mistake.
This was noticed a long time ago. It is just now being re-noticed by others
I discovered this over a year ago while rewriting everything into python:
https://github.com/jgarzik/python-bitcoinlib/
https://github.com/jgarzik/python-bitcoinlib/blob/master/bitcoin/scripteval.py
Can you spot the problem?
I believe Peter created these transactions, and I think that was a good idea. Go Peter!
Speaking to all readers of this thread (not specifically piotr_n) - if your implementation broke because of these transactions, two things to consider:
1) Your implementation almost certainly contains other chain splitting bugs beyond this one.
2) You should be using Matt's blocktester which would have revealed the issue, as Matt discovered this bug >1 year ago when he did the bitcoinj full verification mode.
Speaking to all readers of this thread (not specifically piotr_n) - if your implementation broke because of these transactions, two things to consider:
1) Your implementation almost certainly contains other chain splitting bugs beyond this one.
2) You should be using Matt's blocktester which would have revealed the issue, as Matt discovered this bug >1 year ago when he did the bitcoinj full verification mode.
Live with it ok, but we need to know what is the good behavior!
Doesn't this look like a crazy hack/bug?
Thus my question.Doesn't this look like a crazy hack/bug?
For me it seems like someone has overlooked it at some point; "return 1" was supposed to indicate an error, but it was somehow overlooked and now it implicitly gets cast to (uint256)1 which is a pretty valid hash value, so C++ compiler does not complain and so nobody has noticed..
Don't you have the same problem in Armory?
I cannot imagine reproducing the same behavior in python, just by repeating such a mistake.
Thx.
And sorry for double post.
But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
And sorry for double post.
It only require nIn<=nOut
That's right.But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
Wow I didn't know...
Is it allowed by bitcoin-qt? By the protocol?
I don't find anything in the linked thread
And now we all need to live with it.
Live with it ok, but we need to know what is the good behavior for future implementations!
Doesn't this look like a crazy hack/bug?
The only piece of protocol we have is this:
Quote
Procedure for Hashtype SIGHASH_SINGLE
The output of txCopy is resized to the size of the current input index+1.
All other txCopy outputs aside from the output that is the same as the current input index are set to a blank script and a value of (long) -1.
All other txCopy inputs aside from the current input are set to have an nSequence index of zero.
That requires adding empty outputs until len=index+1, then processingThe output of txCopy is resized to the size of the current input index+1.
All other txCopy outputs aside from the output that is the same as the current input index are set to a blank script and a value of (long) -1.
All other txCopy inputs aside from the current input are set to have an nSequence index of zero.
Is it what bitcoin-qt does?
Thx.
And sorry for double post.
But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
And sorry for double post.
It only require nIn<=nOut
That's right.But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
Wow I didn't know...
Is it allowed by bitcoin-qt? By the protocol?
I don't find anything in the linked thread
And now we all need to live with it - that's another argument on why a documentation other than satoshi's code is useless
Thx.
And sorry for double post.
But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
And sorry for double post.
It only require nIn<=nOut
That's right.But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
Wow I didn't know...
By the protocol?
Thx.
And sorry for double post.
But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
And sorry for double post.
It only require nIn<=nOut
That's right.But what if nIn=2 and nOut=1 - like here?
Well, now I know: use the hash value of 1, but don't dare to fail the script!
Why would SIGHASH_SINGLE require nIn==nOut?
It only require nIn<=nOut
Jump to: