Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consensus: Incorrect conversion from boolean to byte array #1523

Closed
dergoegge opened this issue Aug 19, 2024 · 3 comments
Closed

consensus: Incorrect conversion from boolean to byte array #1523

dergoegge opened this issue Aug 19, 2024 · 3 comments
Labels
Milestone

Comments

@dergoegge
Copy link

Libbitcoin models the script execution stack as a stack of variants (each element is either a int64_t, bool or chunk_xptr). For some operations, the various types need to be converted so that e.g. the elements can be compared.

The conversion of a boolean to a byte array (data_chunk) is currently implemented incorrectly. false is encoded as {0} and true is encoded as {1}:

inline data_chunk chunk::from_bool(bool vary) NOEXCEPT
{
return { bc::to_int<uint8_t>(vary) };
}

According to the reference implementation, false is encoded as {} (empty byte array).

This difference can be demonstrated by the following test case:

libbitcoin test case
diff --git a/test/chain/script.hpp b/test/chain/script.hpp
index 3a8c506f7..c985f8e57 100644
--- a/test/chain/script.hpp
+++ b/test/chain/script.hpp
@@ -521,7 +521,8 @@ const script_test_list valid_context_free_scripts
     { "nop", "nop10 1", "" },
     { "[42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242]", "[2.42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242] equal", "basic push signedness check" },
     { "[1.42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242]", "[2.42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242] equal", "basic pushdata1 signedness check" },
-    { "0x00", "size 0 equal", "basic op_0 execution" }
+    { "0x00", "size 0 equal", "basic op_0 execution" },
+    { "", "0 1 equal size 0 equal", "boolean encoding" }
 }};

 // These are always invalid.
Bitcoin Core test case
diff --git a/src/test/data/tx_valid.json b/src/test/data/tx_valid.json
index 70df0d0f69..0aeaaeec45 100644
--- a/src/test/data/tx_valid.json
+++ b/src/test/data/tx_valid.json
@@ -5,6 +5,9 @@
 ["serializedTransaction, excluded verifyFlags]"],
 ["Objects that are only a single string (like this one) are ignored"],

+["Boolean encoding"],
+[[["0000000000000000000000000000000000000000000000000000000000000000", 0, "0 1 EQUAL SIZE 0 EQUAL"]], "020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0100000000000000000000000000", "STRICTENC,LOW_S,SIGPUSHONLY,MINIMALDATA,DISCOURAGE_UPGRADABLE_NOPS,CLEANSTACK,MINIMALIF,NULLFAIL,DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM,WITNESS_PUBKEYTYPE,CONST_SCRIPTCODE,TAPROOT,DISCOURAGE_UPGRADABLE_PUBKEYTYPE,DISCOURAGE_OP_SUCCESS,DISCOURAGE_UPGRADABLE_TAPROOT_VERSION"],
+
 ["The following is 23b397edccd3740a74adb603c9756370fafcde9bcc4483eb271ecad09a94dd63"],
 ["It is of particular interest because it contains an invalidly-encoded signature which OpenSSL accepts"],
 ["See http://r6.ca/blog/20111119T211504Z.html"],
@evoskuil evoskuil added the bug label Aug 19, 2024
@evoskuil
Copy link
Member

Thanks Niklas!

@evoskuil evoskuil added this to the 4.0 milestone Aug 19, 2024
@evoskuil
Copy link
Member

#1525

@evoskuil
Copy link
Member

Resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants