Report non-mandatory script failures correctly #7276
pull sipa wants to merge 1 commits into bitcoin:master from sipa:nonmanrep changing 1 files +2 −2-
sipa commented at 3:53 pm on January 3, 2016: memberThe same variable was reused for mandatory checking of script failures, so the reported error is always “No error”. Use a new variable check2 so we don’t overwrite the message produced by the first.
-
Report non-mandatory script failures correctly 7ef8f3c072
-
jtimon commented at 7:14 pm on January 3, 2016: contributor
This case only makes sense when in AcceptToMemoryPool anyway, no non-mandatory flgs will be used when called from connectBlock. We could move this special case to AcceptToMemoryPool as well. Something like https://github.com/jtimon/bitcoin/commit/dc0dab8b47b13771dd6bc7ecd08ead7b4e553bee (although that commit is based on previous commits so it wouldn’t be exactly like that if done alone). Maybe we can fix this in that way (also helping libconsensus encapsulation by removing the STANDARD_NOT_MANDATORY_VERIFY_FLAGS [relay policy] dependency from CheckInputs() ).
Or, as always, “that can be done later”. Otherwise utACK, good catch.
-
dcousens commented at 10:43 pm on January 3, 2016: contributorutACK
-
MarcoFalke commented at 1:23 am on January 4, 2016: memberGood catch! utACK 7ef8f3c
-
petertodd commented at 4:32 am on January 4, 2016: contributor
utACK https://github.com/sipa/bitcoin/commit/7ef8f3c072a8750c72a3a1cdc727b5c1d173bac8
Could probably eventually split CheckInputs() into a function for value consistency, fees, etc., and a function that checks the scripts themselves. That’d make running the latter twice with different flags cheap.
-
dcousens commented at 6:16 am on January 4, 2016: contributorActually just ran into this my self, tested ACK @ 7ef8f3c
-
jonasschnelli commented at 7:59 am on January 4, 2016: contributorutACK 7ef8f3c072a8750c72a3a1cdc727b5c1d173bac8
-
jonasschnelli added the label Docs and Output on Jan 4, 2016
-
fanquake commented at 8:07 am on January 4, 2016: member
utACK
On Monday, 4 January 2016, Jonas Schnelli notifications@github.com wrote:
utACK 7ef8f3c https://github.com/bitcoin/bitcoin/commit/7ef8f3c072a8750c72a3a1cdc727b5c1d173bac8
— Reply to this email directly or view it on GitHub #7276 (comment).
-
laanwj commented at 8:11 am on January 4, 2016: memberutACK, this was sneaky
-
laanwj merged this on Jan 4, 2016
-
laanwj closed this on Jan 4, 2016
-
laanwj referenced this in commit d032b5b64b on Jan 4, 2016
-
sipa referenced this in commit 76de36fd2e on Jan 4, 2016
-
jtimon commented at 10:50 am on January 4, 2016: contributor@petertodd perhaphs is now time to reopen #6445 (there’s a more up to date version after a big consensus moveonly in https://github.com/jtimon/bitcoin/commit/babe71589a7ce9e01ce075fc10eb168d5ebc4b12 )? I closed it because “those refactors/optimizations are going to interfere with the mempool limiting work”, but those changes are done now (and it was trivial to rebase, meaning the concern wasn’t really justified).
-
luke-jr referenced this in commit fed8e13099 on Jan 10, 2016
-
luke-jr referenced this in commit eea84de6b0 on Jan 10, 2016
-
zkbot referenced this in commit 84c19b8a87 on Feb 8, 2018
-
zkbot referenced this in commit e1f3a15fdc on Feb 8, 2018
-
zkbot referenced this in commit eb3128ab0a on Feb 19, 2018
-
zkbot referenced this in commit 6db10127a9 on Feb 20, 2018
-
zkbot referenced this in commit 8487be8360 on Feb 20, 2018
-
random-zebra referenced this in commit 5a092159f6 on Aug 5, 2020
-
DrahtBot locked this on Sep 8, 2021
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-18 03:12 UTC
More mirrored repositories can be found on mirror.b10c.me