libconsensus: Add input validation of flags #8976

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2016_10_bitcoinconsensus_input_checking changing 3 files +21 −5

laanwj commented at 6:35 AM on October 20, 2016: member

Makes it an error to use flags that have not been defined on the libconsensus API.

There has been some confusion as to what pass to libconsensus, and (combined with mention in the release notes) this should clear it up.

Using undocumented flags is a risk because their meaning, and what combinations are allowed, changes from release to release. E.g. it is no longer possible to pass (CLEANSTACK | P2SH) without running into an assertion after the segwit changes.

However: this currently fails the tests because our own tests rely on this undocumented behavior(!). I am not entirely sure what would be the best way to solve this. Should we skip tests that have non-supported combinations of flags? Done - this does mean only 322 of 1327 script tests are applied to libconsensus, but those are the ones that test consensus behavior.
laanwj added the label Utils and libraries on Oct 20, 2016
NicolasDorier commented at 6:39 AM on October 20, 2016: contributor

utACK 4522cb4 I claim guilty!
laanwj force-pushed on Oct 20, 2016

libconsensus: Add input validation of flags

Makes it an error to use flags that have not been defined
on the libconsensus API.

There has been some confusion as to what pass to libconsensus, and
(combined with mention in the release notes) this should clear it up.

Using undocumented flags is a risk because their meaning,
and what combinations are allowed, changes from release to release.
E.g. it is no longer possible to pass (CLEANSTACK | P2SH) without
running into an assertion after the segwit changes.

5ca8ef299a

laanwj force-pushed on Oct 20, 2016
sipa commented at 7:08 PM on October 20, 2016: member

Concept ACK.
laanwj merged this on Nov 7, 2016
laanwj closed this on Nov 7, 2016
laanwj referenced this in commit 8c6218a28a on Nov 7, 2016
codablock referenced this in commit 166543805c on Feb 2, 2018
UdjinM6 referenced this in commit 61af31531a on Feb 5, 2018
andvgal referenced this in commit 9d264e7a14 on Jan 6, 2019
CryptoCentric referenced this in commit fc31322a31 on Feb 28, 2019
CryptoCentric referenced this in commit e9706dedb4 on Mar 2, 2019
fanquake referenced this in commit cef2efafce on Dec 7, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#20577 doc: libconsensus: add missing error code description, fix NBitcoin link