OP_SECURETHEBAG Code Review #4
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JeremyRubin
force-pushed
the
securethebag_master
branch
from
ad2178d to
719eada
Compare
JeremyRubin
force-pushed
the
securethebag_master
branch
from
719eada to
8231b82
Compare
JeremyRubin
force-pushed
the
securethebag_master
branch
from
8231b82 to
022c974
Compare
prestwich
reviewed
Oct 21, 2019
| ss = sha256(tag.encode('utf-8')) | ||
| ss += ss | ||
| ss += data | ||
| return sha256(ss) |
prestwich
reviewed
Oct 21, 2019
|
|
||
| SECURETHEBAG_ERROR = "non-mandatory-script-verify-flag (Script failed an OP_SECURETHEBAG operation)" | ||
| def random_bytes(n): | ||
| return bytes(random.getrandbits(8) for i in range(n)) |
prestwich
reviewed
Oct 21, 2019
|
|
||
| BLOCKS = 103 | ||
| self.log.info("Mining %d blocks for mature coinbases", BLOCKS) | ||
| self.coinbase_txids = [self.nodes[0].getblock(b)['tx'][0] for b in self.nodes[0].generate(BLOCKS)] |
There was a problem hiding this comment.
can clean this up by making it an iter and then calling next(self.coinbase_txids) instead of hardcoding indices when accessing
There was a problem hiding this comment.
but does this actually need to be an instance attribute? why not just a local variable?
There was a problem hiding this comment.
So that doesn't quite work I think because we need to generate all the blocks now for maturity, and an iter would be lazy
There was a problem hiding this comment.
you're thinking iter(a for a in b) which would be lazy instead of iter([a for a in b]) which would provide an iterator over the already-evaluated list
There was a problem hiding this comment.
ipython output example:
In [1]: iter([print(i) for i in range(5)])
0
1
2
3
4
Out[1]: <list_iterator at 0x7f172c2cd908>
In [2]: iter(print(i) for i in range(5))
Out[2]: <generator object <genexpr> at 0x7f172c2d5888>
prestwich
reviewed
Oct 21, 2019
| @@ -162,6 +162,11 @@ IsMineResult IsMineInner(const CWallet& keystore, const CScript& scriptPubKey, I | |||
| } | |||
| break; | |||
| } | |||
| case TX_SECURETHEBAG: | |||
| { | |||
| ret = IsMineResult::NO; | |||
There was a problem hiding this comment.
conceptual question: where will this be handled?
There was a problem hiding this comment.
So ismine tells a wallet which things it can spend from. Therefore if you see a TX_SECURETHEBAG, you shouldn't treat it as an output you can spend willy-nilly.
IsMine could be renamed ICanSpendIt, and IsMine could then say yes for TX_SECURETHEBAG
There was a problem hiding this comment.
yah, I'm familiar with the function, I mean do we anticipate nodes will store known Bag info at all? or is that going to be handled solely by external software?
There was a problem hiding this comment.
Yes, if you want to receive a payment, you should store the TX_SECURETHEBAG data in your wallet.
JeremyRubin
pushed a commit
that referenced
this pull request
Oct 23, 2019
…ter-return checking 8d22ab0 ci: Enable address sanitizer (ASan) stack-use-after-return checking (practicalswift) Pull request description: Enable address sanitizer (ASan) stack-use-after-return checking (`detect_stack_use_after_return=1`). Example: ``` #include <iostream> #include <string> const std::string& get_string(int i) { return std::to_string(i); } int main() { std::cout << get_string(41) << "\n"; } ``` Without address sanitizer (ASan) stack-use-after-return checking: ``` $ ./stack-use-after-return $ ``` With address sanitizer (ASan) stack-use-after-return checking: ``` $ ASAN_OPTIONS="detect_stack_use_after_return=1" ./stack-use-after-return ================================================================= ==10400==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f7fa0400030 at pc 0x00000049d2cc bp 0x7ffcbd617070 sp 0x7ffcbd616820 READ of size 2 at 0x7f7abbecd030 thread T0 #0 0x439781 in fwrite #1 0x7f7ac0504cb3 in std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x113cb3) #2 0x4f9b5f in main stack-use-after-return.cpp:9:15 #3 0x7f7abf440b96 in __libc_start_main #4 0x41bbc9 in _start … $ ``` Top commit has no ACKs. Tree-SHA512: 6557a9ff184023380fd9aa433cdf413e01a928ea99dbc59ec138e5d69cb9e13592e8bb5951612f231ff17a37a895bec5c0940c8db5f328a5c840a5771bdeeba5
JeremyRubin
pushed a commit
that referenced
this pull request
Nov 18, 2019
1828c6f refactor: Styling w/ clang-format, comment update (Hennadii Stepanov) 88a94f7 qt: Fix missing qRegisterMetaType for size_t (Hennadii Stepanov) Pull request description: On master (a7aec7a) this connection https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/rpcconsole.cpp#L587 fails due to `ClientModel::mempoolSizeChanged()` signal has unregistered parameter type `size_t`: https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/clientmodel.h#L102 More: ``` $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- -debug=qt ... (lldb) bt * thread bitcoin#17, name = 'QThread', stop reason = signal SIGABRT * frame #0: 0x00007ffff35fce97 libc.so.6`__GI_raise(sig=2) at raise.c:51 frame #1: 0x00007ffff35fe801 libc.so.6`__GI_abort at abort.c:79 frame #2: 0x00007ffff5901352 libQt5Core.so.5`QMessageLogger::warning(char const*, ...) const + 354 frame #3: 0x00007ffff5b216fe libQt5Core.so.5`___lldb_unnamed_symbol2329$$libQt5Core.so.5 + 334 frame #4: 0x00007ffff5b2456d libQt5Core.so.5`QMetaObject::activate(QObject*, int, int, void**) + 1933 frame #5: 0x000055555566872e bitcoin-qt`ClientModel::mempoolSizeChanged(this=<unavailable>, _t1=<unavailable>, _t2=<unavailable>) at moc_clientmodel.cpp:260 ... ``` `debug.log`: ``` [] GUI: QObject::connect: Cannot queue arguments of type 'size_t' (Make sure 'size_t' is registered using qRegisterMetaType().) ``` This PR fixes it. Refs: - [Qt docs: qRegisterMetaType](https://doc.qt.io/qt-5/qmetatype.html#qRegisterMetaType) - bitcoin#16348 --- Side NOTE: Also I believe this line https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/bitcoin.cpp#L63 is redundant since long `CAmount` is a `typedef`. ACKs for top commit: laanwj: Tested ACK 1828c6f Tree-SHA512: 2c7f9fe6a5ae70f2e1dd86b07f95d4b00c85c5706a9d722f063f80beb71880d012ec46556963fb1544c2af53d006936c2f7612eae60d9193f67db62ba3d86129
JeremyRubin
pushed a commit
that referenced
this pull request
Dec 18, 2019
… in Travis 1f9d5af tests: Add initialization order fiasco detection in Travis (practicalswift) Pull request description: Add initialization order fiasco detection in Travis :) Context: bitcoin#17670 (comment) This would have caught the `events_hasher` initialization order issue introduced in bitcoin#17573 and fixed in bitcoin#17670. Output in case of an initialization order fiasco: ``` ==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28 READ of size 8 at 0x557098d79200 thread T0 #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25 #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19 #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5 #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5 #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9 #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59 #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9 #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24 #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac) #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266 #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9) 0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104 registered at: #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3 #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b) SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*) ``` ACKs for top commit: promag: Tested ACK 1f9d5af, got MarcoFalke: ACK 1f9d5af 👔 Tree-SHA512: f24ac0a313df7549193bd7f4fcfdf9b72bdfc6a6ee31d0b08e6d0752e5108fbd532106b6c86377ae0641258c9adb4921872e5d9a0154c0284e03315e0777102c
JeremyRubin
pushed a commit
that referenced
this pull request
Dec 18, 2019
… harness (descriptor_parse) 6338c02 tests: Fix fuzzing harness for descriptor parsing (descriptor_parse) (practicalswift) Pull request description: Fix bug in the descriptor parsing fuzzing harness (`descriptor_parse`) by making sure `secp256k1_context_verify` is properly initialized (via `ECCVerifyHandle`). Background: When fuzzing `Parse(…)` with `libFuzzer` I eventually reached the test case `combo(020000000000000000000000000000000000000000000000000000000000000000)`. That input triggers a call to `CPubKey::IsFullyValid()` which in turns requires an initialized `secp256k1_context_verify`. The fuzzing harness did not fulfil that pre-condition prior to this commit (sorry, my fault!) :) Before: ``` $ mkdir descriptors/ $ echo -n 'combo(020000000000000000000000000000000000000000000000000000000000000000)' > descriptors/input $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" src/test/fuzz/descriptor_parse -runs=1 descriptors/ … pubkey.cpp:210:38: runtime error: null pointer passed as argument 1, which is declared to never be null secp256k1/include/secp256k1.h:305:3: note: nonnull attribute specified here #0 0x561c032ccf25 in CPubKey::IsFullyValid() const src/pubkey.cpp:210:12 #1 0x561c022139c3 in (anonymous namespace)::ParsePubkeyInner(Span<char const> const&, bool, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:674:24 #2 0x561c02207680 in (anonymous namespace)::ParsePubkey(Span<char const> const&, bool, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:730:42 #3 0x561c0220080e in (anonymous namespace)::ParseScript(Span<char const>&, (anonymous namespace)::ParseScriptContext, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:774:23 #4 0x561c021ffb07 in Parse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, bool) src/script/descriptor.cpp:994:16 #5 0x561c0218d5d4 in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/descriptor_parse.cpp:20:9 … $ ``` After: ``` $ mkdir descriptors/ $ echo -n 'combo(020000000000000000000000000000000000000000000000000000000000000000)' > descriptors/input $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" src/test/fuzz/descriptor_parse -runs=1 descriptors/ … Done 2 runs in 0 second(s) $ ``` ACKs for top commit: paymog: ACK 6338c02 MarcoFalke: ACK 6338c02 🕊 Tree-SHA512: bf24c404e1f64183761b057d2f210c3db85277f4415122977c315d7d6835acb5e897b5d64032615e9e44ad4a16dfe857e94481f6e4b57b6dfa8cb37adb2528a5
JeremyRubin
pushed a commit
that referenced
this pull request
Dec 18, 2019
d65fafc gui: disable File->CreateWallet during startup (fanquake) Pull request description: Same as bitcoin#16118. Early calls to Create Wallet will crash bitcoin-qt. ```bash lldb /Applications/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -- --regtest -debug Process 18143 launched: '/Users/michael/github/bitcoin/src/qt/bitcoin-qt' (x86_64) 2019-12-07 15:49:37.823867-0500 bitcoin-qt[18143:5696499] MessageTracer: Falling back to default whitelist Process 18143 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18) frame #0: 0x00000001000d2d9d bitcoin-qt`CreateWalletActivity::createWallet() + 381 bitcoin-qt`CreateWalletActivity::createWallet: -> 0x1000d2d9d <+381>: movq 0x18(%rax), %r14 0x1000d2da1 <+385>: movq %r15, -0xa8(%rbp) 0x1000d2da8 <+392>: leaq -0xa0(%rbp), %r12 0x1000d2daf <+399>: leaq -0x80(%rbp), %rsi Target 0: (bitcoin-qt) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18) * frame #0: 0x00000001000d2d9d bitcoin-qt`CreateWalletActivity::createWallet() + 381 frame #1: 0x0000000100833e6f bitcoin-qt`QMetaObject::activate(QObject*, int, int, void**) + 1631 frame #2: 0x0000000100a1fc47 bitcoin-qt`QDialog::done(int) + 247 frame #3: 0x0000000100833ef5 bitcoin-qt`QMetaObject::activate(QObject*, int, int, void**) + 1765 frame #4: 0x00000001009e04c2 bitcoin-qt`QDialogButtonBoxPrivate::_q_handleButtonClicked() + 786 ``` ACKs for top commit: jonasschnelli: utACK d65fafc promag: ACK d65fafc. Tree-SHA512: 12d7f9e8772508bffbb0163849d9eceec5b1c80068c5d377a4d0973c713dc5f8ad38be8f793fec843d7fb604f0e60a72398b0c95f0a8b775dab39d25b29ac046
JeremyRubin
pushed a commit
that referenced
this pull request
Jul 22, 2020
…lock orders 0ecff9d Improve "detected inconsistent lock order" error message (Hennadii Stepanov) bbe9cf4 test: Improve "potential deadlock detected" exception message (Hennadii Stepanov) 3559934 Fix mistakenly swapped "previous" and "current" lock orders (Hennadii Stepanov) Pull request description: In master (8ef15e8) the "previous" and "current" lock orders are mistakenly swapped. This PR: - fixes printed lock orders - improves the `sync_tests` unit test - makes the "detected inconsistent lock order" error message pointing to the lock location rather `tfm::format()` location. Debugger output example with this PR (with modified code, of course): ``` 2020-06-22T15:46:56Z [msghand] POTENTIAL DEADLOCK DETECTED 2020-06-22T15:46:56Z [msghand] Previous lock order was: 2020-06-22T15:46:56Z [msghand] (2) 'cs_main' in net_processing.cpp:2545 (in thread 'msghand') 2020-06-22T15:46:56Z [msghand] (1) 'g_cs_orphans' in net_processing.cpp:1400 (in thread 'msghand') 2020-06-22T15:46:56Z [msghand] Current lock order is: 2020-06-22T15:46:56Z [msghand] (1) 'g_cs_orphans' in net_processing.cpp:2816 (in thread 'msghand') 2020-06-22T15:46:56Z [msghand] (2) 'cs_main' in net_processing.cpp:2816 (in thread 'msghand') Assertion failed: detected inconsistent lock order for 'cs_main' in net_processing.cpp:2816 (in thread 'msghand'), details in debug log. Process 131393 stopped * thread bitcoin#15, name = 'b-msghand', stop reason = signal SIGABRT frame #0: 0x00007ffff775c18b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 (lldb) bt * thread bitcoin#15, name = 'b-msghand', stop reason = signal SIGABRT * frame #0: 0x00007ffff775c18b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 frame #1: 0x00007ffff773b859 libc.so.6`__GI_abort at abort.c:79:7 frame #2: 0x0000555555e5b196 bitcoind`(anonymous namespace)::potential_deadlock_detected(mismatch=0x00007fff99ff6f30, s1=size=2, s2=size=2, lock_location=0x00007fff99ff7010) at sync.cpp:134:9 frame #3: 0x0000555555e5a1b1 bitcoind`(anonymous namespace)::push_lock(c=0x0000555556379220, locklocation=0x00007fff99ff7010) at sync.cpp:158:13 frame #4: 0x0000555555e59e8a bitcoind`EnterCritical(pszName="cs_main", pszFile="net_processing.cpp", nLine=2816, cs=0x0000555556379220, fTry=false) at sync.cpp:177:5 frame #5: 0x00005555555b0500 bitcoind`UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(this=0x00007fff99ff8c20, pszName="cs_main", pszFile="net_processing.cpp", nLine=2816) at sync.h:134:9 frame #6: 0x00005555555b017f bitcoind`UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(this=0x00007fff99ff8c20, mutexIn=0x0000555556379220, pszName="cs_main", pszFile="net_processing.cpp", nLine=2816, fTry=false) at sync.h:160:13 frame #7: 0x00005555556aa57e bitcoind`ProcessMessage(pfrom=0x00007fff90001180, msg_type=error: summary string parsing error, vRecv=0x00007fff9c005ac0, nTimeReceived=1592840815980751, chainparams=0x00005555564b7110, chainman=0x0000555556380880, mempool=0x0000555556380ae0, connman=0x000055555657aa20, banman=0x00005555565167b0, interruptMsgProc=0x00005555565cae90) at net_processing.cpp:2816:9 ``` ACKs for top commit: laanwj: ACK 0ecff9d vasild: ACK 0ecff9d Tree-SHA512: ff285de8dd3198b5b33c4bfbdadf9b1448189c96143b9696bc4f41c07e784c00851ec169cf3ed45cc325f3617ba6783620803234f57fcce28bf6bc3d6a7234fb
JeremyRubin
pushed a commit
that referenced
this pull request
Nov 13, 2020
…Data fa56d56 fuzz: Properly initialize PrecomputedTransactionData (MarcoFalke) Pull request description: Fixes: ``` script_flags: script/interpreter.cpp:1512: bool SignatureHashSchnorr(uint256 &, const ScriptExecutionData &, const T &, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData &) [T = CTransaction]: Assertion `cache.m_bip341_taproot_ready && cache.m_spent_outputs_ready' failed. ==34989== ERROR: libFuzzer: deadly signal #0 0x55e90077ff11 in __sanitizer_print_stack_trace (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x20cf11) #1 0x55e9006cb068 in fuzzer::PrintStackTrace() (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x158068) #2 0x55e9006b01b3 in fuzzer::Fuzzer::CrashCallback() (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x13d1b3) #3 0x7f6fb89383bf (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf) #4 0x7f6fb855018a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a) #5 0x7f6fb852f858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858) #6 0x7f6fb852f728 (/lib/x86_64-linux-gnu/libc.so.6+0x25728) #7 0x7f6fb8540f35 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x36f35) #8 0x55e9008275bd in bool SignatureHashSchnorr<CTransaction>(uint256&, ScriptExecutionData const&, CTransaction const&, unsigned int, unsigned char, SigVersion, PrecomputedTransactionData const&) /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/script/interpreter.cpp:1512:5 #9 0x55e900825a3f in GenericTransactionSignatureChecker<CTransaction>::CheckSchnorrSignature(Span<unsigned char const>, Span<unsigned char const>, SigVersion, ScriptExecutionData const&, ScriptError_t*) const /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/script/interpreter.cpp:1699:10 #10 0x55e900832503 in VerifyWitnessProgram(CScriptWitness const&, int, std::vector<unsigned char, std::allocator<unsigned char> > const&, unsigned int, BaseSignatureChecker const&, ScriptError_t*, bool) /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/script/interpreter.cpp:1900:26 #11 0x55e90082ecb5 in VerifyScript(CScript const&, CScript const&, CScriptWitness const*, unsigned int, BaseSignatureChecker const&, ScriptError_t*) /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/script/interpreter.cpp:1977:18 #12 0x55e9007a9b61 in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags.cpp:51:30 #13 0x55e9007d0b49 in LLVMFuzzerTestOneInput /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz.cpp:36:5 #14 0x55e9006b1871 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x13e871) bitcoin#15 0x55e9006b0fb5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x13dfb5) bitcoin#16 0x55e9006b38d7 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x1408d7) bitcoin#17 0x55e9006b3c39 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x140c39) bitcoin#18 0x55e9006a290e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x12f90e) bitcoin#19 0x55e9006cb752 in main (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x158752) bitcoin#20 0x7f6fb85310b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) bitcoin#21 0x55e9006776ad in _start (/tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/script_flags+0x1046ad) ACKs for top commit: laanwj: Code review ACK fa56d56 Tree-SHA512: 5637b0a0d982360df32d8cd39e913395967af02ec746508fc5f2fd649695c58bfaaf18ef76f4ca9da764d34fdd63dfe188317dd41b2ed57534bd4055a05ae870
JeremyRubin
pushed a commit
that referenced
this pull request
Dec 16, 2020
8963b2c qt: Improve comments in WalletController::getOrCreateWallet() (Hennadii Stepanov) 5fcfee6 qt: Call setParent() in the parent's context (Hennadii Stepanov) 5659e73 qt: Add ObjectInvoke template function (Hennadii Stepanov) Pull request description: The `setParent(parent)` internally calls `QCoreApplication::sendEvent(parent, QChildEvent)` that implies running in the thread which created the parent object. That is not the case always, and an internal assertion fails in the debug mode. Steps to reproduce this issue on master (007e15d) on Linux Mint 20 (x86_64): ``` $ make -C depends DEBUG=1 $ CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure $ make $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- --regtest -debug=qt (lldb) target create "src/qt/bitcoin-qt" Current executable set to '/home/hebasto/GitHub/bitcoin/src/qt/bitcoin-qt' (x86_64). (lldb) settings set -- target.run-args "--regtest" "-debug=qt" (lldb) run Process 431562 launched: '/home/hebasto/GitHub/bitcoin/src/qt/bitcoin-qt' (x86_64) # load wallet via GUI Process 431562 stopped * thread bitcoin#24, name = 'QThread', stop reason = signal SIGABRT frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 (lldb) bt * thread bitcoin#24, name = 'QThread', stop reason = signal SIGABRT * frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 frame #1: 0x00007ffff7924859 libc.so.6`__GI_abort at abort.c:79:7 frame #2: 0x0000555556508ec4 bitcoin-qt`::qt_message_fatal((null)=<unavailable>, context=<unavailable>, message=<unavailable>) at qlogging.cpp:1690:15 frame #3: 0x00005555565099cf bitcoin-qt`QMessageLogger::fatal(this=<unavailable>, msg=<unavailable>) const at qlogging.cpp:796:21 frame #4: 0x000055555650479d bitcoin-qt`qt_assert_x(where=<unavailable>, what=<unavailable>, file=<unavailable>, line=<unavailable>) at qglobal.cpp:3088:46 frame #5: 0x0000555556685733 bitcoin-qt`QCoreApplicationPrivate::checkReceiverThread(receiver=0x0000555557b27510) at qcoreapplication.cpp:557:5 frame #6: 0x00005555567ced86 bitcoin-qt`QApplication::notify(this=0x00007fffffffd4a0, receiver=0x0000555557b27510, e=0x00007fff9a7f8ce0) at qapplication.cpp:2956:27 frame #7: 0x0000555556685d31 bitcoin-qt`QCoreApplication::notifyInternal2(receiver=0x0000555557b27510, event=0x00007fff9a7f8ce0) at qcoreapplication.cpp:1024:24 frame #8: 0x00005555566c9224 bitcoin-qt`QObjectPrivate::setParent_helper(QObject*) [inlined] QCoreApplication::sendEvent(event=<unavailable>, receiver=<unavailable>) at qcoreapplication.h:233:59 frame #9: 0x00005555566c9210 bitcoin-qt`QObjectPrivate::setParent_helper(this=0x00007fff85855260, o=0x0000555557b27510) at qobject.cpp:2036 frame #10: 0x00005555566c9b41 bitcoin-qt`QObject::setParent(this=<unavailable>, parent=<unavailable>) at qobject.cpp:1980:24 frame #11: 0x0000555555710be8 bitcoin-qt`WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet, std::default_delete<interfaces::Wallet> >) + 2534 ... ``` Fixes bitcoin#18835. ACKs for top commit: ryanofsky: Code review ACK 8963b2c. No changes since last review, just rebase because of conflict on some adjacent lines jonasschnelli: utACK 8963b2c Tree-SHA512: fef615904168717df3d8a0bd85eccc3eef990cc3e66c9fa280c8ef08ea009a7cb5a2a4f868ed0be3c0fe5bf683e8465850b5958deb896fdadd22d296186c9586
JeremyRubin
pushed a commit
that referenced
this pull request
Mar 17, 2021
fa6c114 test: Add sanitizer suppressions for AMD EPYC CPUs (MarcoFalke) Pull request description: Currently the ci system only runs on intel cpus (and some arm devices), but it won't run on CPUs `Using the 'shani(1way,2way)' SHA256 implementation` (excerpt from debug log). For reference, google cloud CPUs (which is what Cirrus CI uses) print `Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation` The traceback I got: ``` crypto/sha256_shani.cpp:87:18: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long') #0 0x55c0000e95ec in sha256_shani::Transform(unsigned int*, unsigned char const*, unsigned long) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256_shani.cpp:87:18 #1 0x55bfffb926f8 in (anonymous namespace)::SelfTest() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:517:9 #2 0x55bfffb906ed in SHA256AutoDetect[abi:cxx11]() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:626:5 #3 0x55bfff87ab97 in BasicTestingSetup::BasicTestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/util/setup_common.cpp:104:5 #4 0x55bffe885877 in main /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_main.cpp:52:27 #5 0x7f20c3bf60b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) #6 0x55bffe7a5f6d in _start (/root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_bitcoin-qt+0x1d00f6d) SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow crypto/sha256_shani.cpp:87:18 in ACKs for top commit: laanwj: Anyhow ACK fa6c114 Tree-SHA512: 968a1d28eedec58c337b1323862f583cb1bcd78c5f03396940b9ab53ded12f8c6652877909aba05ee5586532137418fd817ff979bd7bef6e07856094f9d7f9b1
JeremyRubin
pushed a commit
that referenced
this pull request
Apr 6, 2021
…-overflow:txmempool.cpp fad8a97 test: Add missing suppression for signed-integer-overflow:txmempool.cpp (MarcoFalke) Pull request description: Otherwise the fuzzer will crash: ``` txmempool.cpp:847:15: runtime error: signed integer overflow: 8138645194045128704 + 4611686018427387904 cannot be represented in type 'long' #0 0x558ff1838d4c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /root/fuzz_dir/scratch/fuzz_gen/code/src/txmempool.cpp:847:15 #1 0x558ff196e723 in LoadMempool(CTxMemPool&, CChainState&, std::function<_IO_FILE* (boost::filesystem::path const&, char const*)>) /root/fuzz_dir/scratch/fuzz_gen/code/src/validation.cpp:5053:22 #2 0x558ff13f37ab in validation_load_mempool_fuzz_target(Span<unsigned char const>) /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/validation_load_mempool.cpp:32:11 #3 0x558ff1083378 in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2 #4 0x558ff22a749d in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 #5 0x558ff22a70e8 in LLVMFuzzerTestOneInput /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz.cpp:63:5 #6 0x558ff0f83543 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o #7 0x558ff0f6d442 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o #8 0x558ff0f7323a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o #9 0x558ff0f9ef82 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x196df82) #10 0x7f1237f310b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 #11 0x558ff0f4816d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x191716d) Top commit has no ACKs. Tree-SHA512: 94c13771054b4acfb83e3dcfa09beb3f9d0ca0e025d3993cdf2e46df6456f227565b31fd4377b8dd86c567aeee800f293ac57a470c6f5f81e9177d460e7bd705
JeremyRubin
pushed a commit
that referenced
this pull request
Jun 26, 2021
Otherwise it is not possible to run bench_bitcoin with clang-12 + ASAN
compiled.
Output:
$ src/bench/bench_bitcoin
bench/nanobench.h:1107:15: runtime error: left shift of 4982565676696827473 by 27 places cannot be represented in type 'uint64_t' (aka 'unsigned long')
#0 0x5623d6a13137 in ankerl::nanobench::Rng::rotl(unsigned long, unsigned int) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./bench/nanobench.h:1107:15
#1 0x5623d6a13137 in ankerl::nanobench::Rng::operator()() /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./bench/nanobench.h:1075:10
#2 0x5623d6a05c5b in ankerl::nanobench::Rng::Rng(unsigned long) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./bench/nanobench.h:3135:9
#3 0x5623d6a0ca51 in ankerl::nanobench::detail::IterationLogic::Impl::Impl(ankerl::nanobench::Bench const&) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./bench/nanobench.h:2206:13
#4 0x5623d69f8f73 in ankerl::nanobench::detail::IterationLogic::IterationLogic(ankerl::nanobench::Bench const&) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./bench/nanobench.h:2215:18
#5 0x5623d690f165 in ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<AddrManAdd(ankerl::nanobench::Bench&)::$_0>(AddrManAdd(ankerl::nanobench::Bench&)::$_0&&) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./bench/nanobench.h:1114:28
#6 0x5623d690e26e in AddrManAdd(ankerl::nanobench::Bench&) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bench/addrman.cpp:76:11
#7 0x5623d69279d6 in void std::__invoke_impl<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(std::__invoke_other, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14
#8 0x5623d6927921 in std::enable_if<is_invocable_r_v<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>, void>::type std::__invoke_r<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:110:2
#9 0x5623d692775f in std::_Function_handler<void (ankerl::nanobench::Bench&), void (*)(ankerl::nanobench::Bench&)>::_M_invoke(std::_Any_data const&, ankerl::nanobench::Bench&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:291:9
#10 0x5623d692dbd5 in std::function<void (ankerl::nanobench::Bench&)>::operator()(ankerl::nanobench::Bench&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:622:14
#11 0x5623d692cd44 in benchmark::BenchRunner::RunAll(benchmark::Args const&) /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bench/bench.cpp:65:13
#12 0x5623d69282bf in main /bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bench/bench_bitcoin.cpp:63:5
#13 0x7f6812010564 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28564)
#14 0x5623d685f4dd in _start (/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bench/bench_bitcoin+0x13754dd)
SUMMARY: UndefinedBehaviorSanitizer: invalid-shift-base bench/nanobench.h:1107:15 in
$ clang --version
Ubuntu clang version 12.0.0-1ubuntu1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
JeremyRubin
pushed a commit
that referenced
this pull request
Jun 26, 2021
fa1fdeb fuzz: Ensure prevout is consensus-valid (MarcoFalke) Pull request description: It shouldn't be possible to create consensus-invalid prevouts, so there is no need to fuzz them. To reproduce: ``` $ echo 'AAAAAAEAAAAAAQF0Rw0SGsrit4+YZSEfpcQT/o+bJbjgVjATUHqrCfRE+QsBAAAXFgAUlsvXHgGV ZxF3QXxitwe1tIOYdLj2NePHATl9CgAAAAAAGXapFOFHg1yqRFl7soeowwpIEOoe9G1NiKwCRzBE AiAx6F2Q008gvJnok6JiyOn7lPqCJJmDiI2omRNXT1Q7XAIgCQP6WJizAqhnvImpQqYMJkqePGvx Jy/pGRMy1iNL0ecDIQJr4tWomVTBfjpyMFMOD9aDAR5gkByOIYiaQOv8P/sRztP3pS8RDAAAEUUE NQBwYAAAAAC5F6kUTLIzj/lKP2Hmpwyzukns2eweRkOH' | base64 --decode > /tmp/a $ FUZZ=script_flags ./src/test/fuzz/fuzz /tmp/a INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 59714236 INFO: Loaded 1 modules (212532 inline 8-bit counters): 212532 [0x55987fb3f668, 0x55987fb7349c), INFO: Loaded 1 PC tables (212532 PCs): 212532 [0x55987fb734a0,0x55987feb17e0), ./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each. Running: /tmp/a fuzz: script/interpreter.cpp:1495: bool HandleMissingData(MissingDataBehavior): Assertion `!"Missing data"' failed. ==520092== ERROR: libFuzzer: deadly signal #0 0x55987f111180 in __sanitizer_print_stack_trace (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5ea180) #1 0x55987f0ba828 in fuzzer::PrintStackTrace() fuzzer.o #2 0x55987f09de43 in fuzzer::Fuzzer::CrashCallback() fuzzer.o #3 0x7fd003d563bf (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf) #4 0x7fd00399a18a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a) #5 0x7fd003979858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858) #6 0x7fd003979728 (/lib/x86_64-linux-gnu/libc.so.6+0x25728) #7 0x7fd00398af35 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x36f35) #8 0x55987f8ce194 in HandleMissingData(MissingDataBehavior) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:1495:9 #9 0x55987f8ce194 in GenericTransactionSignatureChecker<CTransaction>::CheckECDSASignature(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, CScript const&, SigVersion) const /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:1685:68 #10 0x55987f8cbbc8 in EvalChecksigPreTapscript(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, prevector<28u, unsigned char, unsigned int, int>::const_iterator, prevector<28u, unsigned char, unsigned int, int>::const_iterator, unsigned int, BaseSignatureChecker const&, SigVersion, ScriptError_t*, bool&) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:363:24 #11 0x55987f8cbbc8 in EvalChecksig(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, prevector<28u, unsigned char, unsigned int, int>::const_iterator, prevector<28u, unsigned char, unsigned int, int>::const_iterator, ScriptExecutionData&, unsigned int, BaseSignatureChecker const&, SigVersion, ScriptError_t*, bool&) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:421:16 #12 0x55987f8c5a01 in EvalScript(std::vector<std::vector<unsigned char, std::allocator<unsigned char> >, std::allocator<std::vector<unsigned char, std::allocator<unsigned char> > > >&, CScript const&, unsigned int, BaseSignatureChecker const&, SigVersion, ScriptExecutionData&, ScriptError_t*) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:1094:26 #13 0x55987f8d6d6e in ExecuteWitnessScript(Span<std::vector<unsigned char, std::allocator<unsigned char> > const> const&, CScript const&, unsigned int, SigVersion, BaseSignatureChecker const&, ScriptExecutionData&, ScriptError_t*) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:1843:10 #14 0x55987f8d48fc in VerifyWitnessProgram(CScriptWitness const&, int, std::vector<unsigned char, std::allocator<unsigned char> > const&, unsigned int, BaseSignatureChecker const&, ScriptError_t*, bool) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:1904:20 bitcoin#15 0x55987f8d3d8b in VerifyScript(CScript const&, CScript const&, CScriptWitness const*, unsigned int, BaseSignatureChecker const&, ScriptError_t*) /root/fuzz_dir/scratch/fuzz_gen/code/src/script/interpreter.cpp:2045:22 bitcoin#16 0x55987f201d47 in script_flags_fuzz_target(Span<unsigned char const>) /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/script_flags.cpp:54:30 bitcoin#17 0x55987f11447f in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2 bitcoin#18 0x55987f8aed17 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 bitcoin#19 0x55987f8aed17 in LLVMFuzzerTestOneInput /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz.cpp:63:5 bitcoin#20 0x55987f09f5e3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o bitcoin#21 0x55987f0894e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o bitcoin#22 0x55987f08f2da in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o bitcoin#23 0x55987f0bb002 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x594002) bitcoin#24 0x7fd00397b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) bitcoin#25 0x55987f06420d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x53d20d) NOTE: libFuzzer has rudimentary signal handlers. Combine libFuzzer with AddressSanitizer or similar for better crash reports. SUMMARY: libFuzzer: deadly signal ACKs for top commit: practicalswift: Tested ACK fa1fdeb Tree-SHA512: 6130ed9ab6d8eeab901f64a1c069300e67d0b6009c42763262fe6edeab8192e088c1a3c1f61aee900b9ebbc48fbf6e837b41704bad592ec526398355766e208a
JeremyRubin
pushed a commit
that referenced
this pull request
Jun 26, 2021
…ferenceable iterator outside its valid range) fa09871 refactor: Avoid sign-compare compiler warning in util/asmap (MarcoFalke) Pull request description: Can be reproduced on current master with `D_GLIBCXX_DEBUG`: ``` /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/debug/safe_iterator.h:883: In function: __gnu_debug::_Safe_iterator<type-parameter-0-0, type-parameter-0-1, std::random_access_iterator_tag>::_Self __gnu_debug::operator+(const __gnu_debug::_Safe_iterator<type-parameter-0-0, type-parameter-0-1, std::random_access_iterator_tag>::_Self &, __gnu_debug::_Safe_iterator<type-parameter-0-0, type-parameter-0-1, std::random_access_iterator_tag>::difference_type) Error: attempt to advance a dereferenceable iterator 369 steps, which falls outside its valid range. Objects involved in the operation: iterator @ 0x0x7ffd3d613138 { type = std::__cxx1998::_Bit_const_iterator (constant iterator); state = dereferenceable; references sequence with type 'std::__debug::vector<bool, std::allocator<bool> >' @ 0x0x7ffd3d663590 } ==65050== ERROR: libFuzzer: deadly signal #0 0x559ab9787690 in __sanitizer_print_stack_trace (/bitcoin/src/test/fuzz/fuzz+0x5a1690) #1 0x559ab9733998 in fuzzer::PrintStackTrace() (/bitcoin/src/test/fuzz/fuzz+0x54d998) #2 0x559ab9718ae3 in fuzzer::Fuzzer::CrashCallback() (/bitcoin/src/test/fuzz/fuzz+0x532ae3) #3 0x7f70a0e723bf (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf) #4 0x7f70a0b3418a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a) #5 0x7f70a0b13858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858) #6 0x7f70a0f21148 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xa1148) #7 0x559ab9f60a96 in __gnu_debug::operator+(__gnu_debug::_Safe_iterator<std::__cxx1998::_Bit_const_iterator, std::__debug::vector<bool, std::allocator<bool> >, std::random_access_iterator_tag> const&, long) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/debug/safe_iterator.h:881:2 #8 0x559ab9f61062 in SanityCheckASMap(std::__debug::vector<bool, std::allocator<bool> > const&, int) util/asmap.cpp:159:21 #9 0x559ab9e4fdfa in SanityCheckASMap(std::__debug::vector<bool, std::allocator<bool> > const&) netaddress.cpp:1242:12 #10 0x559ab9793fcb in addrman_fuzz_target(Span<unsigned char const>) test/fuzz/addrman.cpp:43:14 #11 0x559ab978a03c in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2 #12 0x559aba2692c7 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 #13 0x559aba269132 in LLVMFuzzerTestOneInput test/fuzz/fuzz.cpp:63:5 #14 0x559ab971a1a1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/bitcoin/src/test/fuzz/fuzz+0x5341a1) bitcoin#15 0x559ab97198e5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/bitcoin/src/test/fuzz/fuzz+0x5338e5) bitcoin#16 0x559ab971bb87 in fuzzer::Fuzzer::MutateAndTestOne() (/bitcoin/src/test/fuzz/fuzz+0x535b87) bitcoin#17 0x559ab971c885 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/bitcoin/src/test/fuzz/fuzz+0x536885) bitcoin#18 0x559ab970b23e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/bitcoin/src/test/fuzz/fuzz+0x52523e) bitcoin#19 0x559ab9734082 in main (/bitcoin/src/test/fuzz/fuzz+0x54e082) bitcoin#20 0x7f70a0b150b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) bitcoin#21 0x559ab96dffdd in _start (/bitcoin/src/test/fuzz/fuzz+0x4f9fdd) ACKs for top commit: sipa: utACK fa09871 vasild: ACK fa09871 Tree-SHA512: 802fda33bda40fe2521f1e3be075ceddc5fd9ba185bd494286e50019931dfd688da7a6513601138b1dc7bb8e80ae47c8572902406eb59f68990619ddb2656748
JeremyRubin
pushed a commit
that referenced
this pull request
Jun 26, 2021
fa340b8 refactor: Avoid magic value of all-zeros in assumeutxo base_blockhash (MarcoFalke) fae33f9 Fix assumeutxo crash due to invalid base_blockhash (MarcoFalke) fa5668b refactor: Use type-safe assumeutxo hash (MarcoFalke) 0000007 refactor: Remove unused code (MarcoFalke) faa921f move-only: Add util/hash_type (MarcoFalke) Pull request description: Starting with commit d6af06d, a block hash of all-zeros is invalid and will lead to a crash of the node. Can be tested by cherry-picking the test changes without the other changes. Stack trace (copied from bitcoin#21584 (comment)): ``` #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x00007ffff583c8b1 in __GI_abort () at abort.c:79 #2 0x00007ffff582c42a in __assert_fail_base (fmt=0x7ffff59b3a38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x555556c8b450 "!hashBlock.IsNull()", file=file@entry=0x555556c8b464 "txdb.cpp", line=line@entry=89, function=function@entry=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:92 #3 0x00007ffff582c4a2 in __GI___assert_fail (assertion=0x555556c8b450 "!hashBlock.IsNull()", file=0x555556c8b464 "txdb.cpp", line=89, function=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:101 #4 0x000055555636738b in CCoinsViewDB::BatchWrite (this=0x5555577975c0, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at txdb.cpp:89 #5 0x00005555564a2e80 in CCoinsViewBacked::BatchWrite (this=0x5555577975f8, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at coins.cpp:30 #6 0x00005555564a43de in CCoinsViewCache::Flush (this=0x55555778eaf0) at coins.cpp:223 #7 0x00005555563fc11d in ChainstateManager::PopulateAndValidateSnapshot (this=0x55555740b038 <g_chainman>, snapshot_chainstate=..., coins_file=..., metadata=...) at validation.cpp:5422 #8 0x00005555563fab3d in ChainstateManager::ActivateSnapshot (this=0x55555740b038 <g_chainman>, coins_file=..., metadata=..., in_memory=true) at validation.cpp:5299 #9 0x0000555555e8c893 in validation_chainstatemanager_tests::CreateAndActivateUTXOSnapshot<validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12>(NodeContext&, boost::filesystem::path, validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12) (node=..., root=..., malleation=...) at test/validation_chainstatemanager_tests.cpp:199 #10 0x0000555555e8877a in validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method (this=0x7fffffffc8d0) at test/validation_chainstatemanager_tests.cpp:262 ACKs for top commit: laanwj: Code review re-ACK fa340b8 jamesob: ACK fa340b8 ([`jamesob/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due`](https://github.com/jamesob/bitcoin/tree/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due)) Tree-SHA512: c2c4e66c1abfd400ef18a04f22fec1f302f1ff4d27a18050f492f688319deb4ccdd165ff792eee0a1f816e7b69fb64080662b79517ab669e3d26b9eb77802851
JeremyRubin
pushed a commit
that referenced
this pull request
Jun 26, 2021
fa2e614 test: Fix off-by-one in mockscheduler test RPC (MarcoFalke) Pull request description: Fixes: ``` fuzz: scheduler.cpp:83: void CScheduler::MockForward(std::chrono::seconds): Assertion `delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1}' failed. ==1059066== ERROR: libFuzzer: deadly signal #0 0x558f75449c10 in __sanitizer_print_stack_trace (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5fec10) #1 0x558f753f32b8 in fuzzer::PrintStackTrace() fuzzer.o #2 0x558f753d68d3 in fuzzer::Fuzzer::CrashCallback() fuzzer.o #3 0x7f4a3cbbb3bf (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf) #4 0x7f4a3c7ff18a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a) #5 0x7f4a3c7de858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858) #6 0x7f4a3c7de728 (/lib/x86_64-linux-gnu/libc.so.6+0x25728) #7 0x7f4a3c7eff35 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x36f35) #8 0x558f7588a913 in CScheduler::MockForward(std::chrono::duration<long, std::ratio<1l, 1l> >) scheduler.cpp:83:5 #9 0x558f75b0e5b1 in mockscheduler()::$_7::operator()(RPCHelpMan const&, JSONRPCRequest const&) const rpc/misc.cpp:435:30 #10 0x558f75b0e5b1 in std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), mockscheduler()::$_7>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9 #11 0x558f7587a141 in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 #12 0x558f7587a141 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) const rpc/util.cpp:565:26 #13 0x558f756c0086 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const ./rpc/server.h:110:91 #14 0x558f756c0086 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9 bitcoin#15 0x558f756b8592 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 bitcoin#16 0x558f756b8592 in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) rpc/server.cpp:480:20 bitcoin#17 0x558f756b8592 in ExecuteCommands(std::vector<CRPCCommand const*, std::allocator<CRPCCommand const*> > const&, JSONRPCRequest const&, UniValue&) rpc/server.cpp:444:13 bitcoin#18 0x558f756b8017 in CRPCTable::execute(JSONRPCRequest const&) const rpc/server.cpp:464:13 bitcoin#19 0x558f7552457a in (anonymous namespace)::RPCFuzzTestingSetup::CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) test/fuzz/rpc.cpp:50:25 bitcoin#20 0x558f7552457a in rpc_fuzz_target(Span<unsigned char const>) test/fuzz/rpc.cpp:354:28 bitcoin#21 0x558f7544cf0f in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2 bitcoin#22 0x558f75c05197 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 bitcoin#23 0x558f75c05197 in LLVMFuzzerTestOneInput test/fuzz/fuzz.cpp:74:5 bitcoin#24 0x558f753d8073 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o bitcoin#25 0x558f753c1f72 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o bitcoin#26 0x558f753c7d6a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o bitcoin#27 0x558f753f3a92 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5a8a92) bitcoin#28 0x7f4a3c7e00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) bitcoin#29 0x558f7539cc9d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x551c9d) ACKs for top commit: practicalswift: cr ACK fa2e614 Tree-SHA512: cfa120265261f0ad019b46c426b915c1c007806b37aecb27016ce780a0ddea5e6fc9b09065fd40684b11183dcd3bf543558d7a655e604695021653540266baf7
JeremyRubin
pushed a commit
that referenced
this pull request
Jan 19, 2022
This is needed to turn globals into member variables. Otherwise, this
will lead to issues:
runtime error: reference binding to null pointer of type 'CBlockFileInfo'
#0 in std::vector<CBlockFileInfo, std::allocator<CBlockFileInfo> >::operator[](unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:1046:2
#1 in BlockManager::FlushBlockFile(bool, bool) src/node/blockstorage.cpp:540:47
#2 in CChainState::FlushStateToDisk(BlockValidationState&, FlushStateMode, int) src/validation.cpp:2262:28
#3 in CChainState::ResizeCoinsCaches(unsigned long, unsigned long) src/validation.cpp:4414:15
#4 in validation_chainstate_tests::validation_chainstate_resize_caches::test_method() src/test/validation_chainstate_tests.cpp:66:12
JeremyRubin
pushed a commit
that referenced
this pull request
Feb 17, 2022
fa45597 util: Add missing unlinkat to syscall sandbox (MarcoFalke) Pull request description: This will be needed for g++-12 (after libstdc++6 12-20220206). Steps to reproduce: ``` gdb --args ./src/bitcoind -sandbox=log-and-abort -regtest ./src/bitcoin-cli -regtest -named createwallet wallet_name=a descriptors=false ./src/bitcoin-cli -regtest stop ``` BT: ``` Thread 1 "b-shutoff" received signal SIGSYS, Bad system call. 0x00007ffff79564f7 in unlinkat () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) bt #0 0x00007ffff79564f7 in unlinkat () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007ffff7cc7335 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #2 0x00007ffff7cc94e3 in std::filesystem::remove_all(std::filesystem::__cxx11::path const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #3 0x00005555559d4918 in wallet::BerkeleyEnvironment::Flush (this=0x7fffc4005160, fShutdown=<optimized out>) at /usr/include/c++/12/bits/fs_path.h:595 #4 0x000055555592c058 in wallet::StopWallets (context=...) at /usr/include/c++/12/bits/shared_ptr_base.h:1665 #5 0x00005555556617ca in Shutdown (node=...) at ./src/init.cpp:293 #6 0x000055555563ada6 in AppInit (argv=<optimized out>, argc=<optimized out>, node=...) at ./src/bitcoind.cpp:249 #7 main (argc=<optimized out>, argv=<optimized out>) at ./src/bitcoind.cpp:273 ACKs for top commit: laanwj: Code review ACK fa45597 Tree-SHA512: e80a38828f8656040954c9befa2d1c9d5170e204dc09c61031633349897f51ccd85cc5c99a089c4726d7f5237875cd9ed3fa8ef864cd6c1c8a2b8250b392d57f
JeremyRubin
pushed a commit
that referenced
this pull request
Feb 28, 2022
fa7e147 test: Fix intermittent Tsan issue (MarcoFalke) Pull request description: Fix https://cirrus-ci.com/task/5176769937408000?logs=ci#L5161 ``` WARNING: ThreadSanitizer: data race (pid=22965) Write of size 8 at 0x7f74d5e21f50 by main thread: #0 std::__1::ios_base::precision(long) /usr/lib/llvm-13/bin/../include/c++/v1/ios:513:18 (test_bitcoin+0x1a8366) #1 boost::io::ios_base_all_saver::restore() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/io/ios_state.hpp:341:17 (test_bitcoin+0x1a8366) #2 boost::unit_test::unit_test_log_t::operator<<(boost::unit_test::log::begin const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_log.ipp:336:55 (test_bitcoin+0x1a8366) #3 boost::test_tools::tt_detail::report_assertion(boost::test_tools::assertion_result const&, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, unsigned long, ...) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/test_tools.ipp:359:19 (test_bitcoin+0x1b3b9b) #4 txindex_tests::txindex_initial_sync::test_method() src/test/txindex_tests.cpp:31:5 (test_bitcoin+0x78aebc) #5 txindex_tests::txindex_initial_sync_invoker() src/test/txindex_tests.cpp:16:1 (test_bitcoin+0x78a384) #6 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bf30d) #7 boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x224027) #8 boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1368:32 (test_bitcoin+0x224027) #9 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x224027) #10 boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ac66c) #11 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:290:30 (test_bitcoin+0x1ac66c) #12 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:879:16 (test_bitcoin+0x1ac66c) #13 boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1277:16 (test_bitcoin+0x1ac980) #14 boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1377:5 (test_bitcoin+0x1a7f9b) bitcoin#15 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1a7f9b) bitcoin#16 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:823:44 (test_bitcoin+0x1e0d5c) bitcoin#17 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6) bitcoin#18 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6) bitcoin#19 boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1696:29 (test_bitcoin+0x1a6bfb) bitcoin#20 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:248:9 (test_bitcoin+0x1c4ed6) bitcoin#21 main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:304:12 (test_bitcoin+0x1c5506) Previous write of size 8 at 0x7f74d5e21f50 by thread T4: [failed to restore the stack] Location is global 'std::__1::cout' of size 160 at 0x7f74d5e21f30 (libc++.so.1+0x0000000cdf50) Thread T4 'b-txindex' (tid=22989, running) created by main thread at: #0 pthread_create <null> (test_bitcoin+0x1184cd) #1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:514:10 (test_bitcoin+0xa23f1b) #2 std::__1::thread::thread<void (*)(char const*, std::__1::function<void ()>), char const*, BaseIndex::Start(CChainState&)::$_0, void>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, BaseIndex::Start(CChainState&)::$_0&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (test_bitcoin+0xa23f1b) #3 BaseIndex::Start(CChainState&) src/index/base.cpp:363:21 (test_bitcoin+0xa23f1b) #4 txindex_tests::txindex_initial_sync::test_method() src/test/txindex_tests.cpp:31:5 (test_bitcoin+0x78adfa) #5 txindex_tests::txindex_initial_sync_invoker() src/test/txindex_tests.cpp:16:1 (test_bitcoin+0x78a384) #6 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bf30d) #7 boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x224027) #8 boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1368:32 (test_bitcoin+0x224027) #9 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x224027) #10 boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ac66c) #11 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:290:30 (test_bitcoin+0x1ac66c) #12 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:879:16 (test_bitcoin+0x1ac66c) #13 boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1277:16 (test_bitcoin+0x1ac980) #14 boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1377:5 (test_bitcoin+0x1a7f9b) bitcoin#15 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1a7f9b) bitcoin#16 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:823:44 (test_bitcoin+0x1e0d5c) bitcoin#17 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6) bitcoin#18 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6) bitcoin#19 boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1696:29 (test_bitcoin+0x1a6bfb) bitcoin#20 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:248:9 (test_bitcoin+0x1c4ed6) bitcoin#21 main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:304:12 (test_bitcoin+0x1c5506) SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/ios:513:18 in std::__1::ios_base::precision(long) ================== Exit status: 2 ACKs for top commit: fanquake: CI ignored ACK fa7e147 Tree-SHA512: 5194e026410b96ad3c8addeecce0a55ee0271c3cfac9fa0715345b1a50d59925549cee0a3e415e5837ae6d2f214a7b622c73cfc7fdf41d5e55c24fb87fddb9d1
JeremyRubin
pushed a commit
that referenced
this pull request
Dec 17, 2022
fac04cb refactor: Add lock annotations to Active* methods (MacroFake) fac15ff Fix logical race in rest_getutxos (MacroFake) fa97a52 Fix UB/data-race in RPCNotifyBlockChange (MacroFake) fa530bc Add ChainstateManager::GetMutex(), an alias for ::cs_main (MacroFake) Pull request description: This fixes two issues: * A data race in `ActiveChain`, which returns a reference to the chain (a `std::vector`), which is not thread safe. See also below traceback. * A corrupt rest response, which returns a blockheight and blockhash, which are unrelated to each other and to the result, as the chain might advance between each call without cs_main held. The issues are fixed by taking cs_main and holding it for the required time. ``` ================== WARNING: ThreadSanitizer: data race (pid=32335) Write of size 8 at 0x7b3c000008f0 by thread T22 (mutexes: write M131626, write M151, write M131553): #0 std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&) /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 (bitcoind+0x501239) #1 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__swap_out_circular_buffer(std::__1::__split_buffer<CBlockIndex*, std::__1::allocator<CBlockIndex*>&>&) /usr/lib/llvm-13/bin/../include/c++/v1/vector:977:5 (bitcoind+0x501239) #2 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__append(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:1117:9 (bitcoind+0x501239) #3 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::resize(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:2046:15 (bitcoind+0x4ffe29) #4 CChain::SetTip(CBlockIndex*) src/chain.cpp:19:12 (bitcoind+0x4ffe29) #5 CChainState::ConnectTip(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, ConnectTrace&, DisconnectedBlockTransactions&) src/validation.cpp:2748:13 (bitcoind+0x475d00) #6 CChainState::ActivateBestChainStep(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, bool&, ConnectTrace&) src/validation.cpp:2884:18 (bitcoind+0x47739e) #7 CChainState::ActivateBestChain(BlockValidationState&, std::__1::shared_ptr<CBlock const>) src/validation.cpp:3011:22 (bitcoind+0x477baf) #8 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74) #9 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e) #10 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e) #11 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e) #12 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e) #13 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e) #14 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f) bitcoin#15 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f) bitcoin#16 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f) bitcoin#17 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a) bitcoin#18 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a) bitcoin#19 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a) Previous read of size 8 at 0x7b3c000008f0 by main thread: #0 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::size() const /usr/lib/llvm-13/bin/../include/c++/v1/vector:680:61 (bitcoind+0x15179d) #1 CChain::Tip() const src/./chain.h:449:23 (bitcoind+0x15179d) #2 ChainstateManager::ActiveTip() const src/./validation.h:927:59 (bitcoind+0x15179d) #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1841:35 (bitcoind+0x15179d) #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) Location is heap block of size 232 at 0x7b3c00000870 allocated by main thread: #0 operator new(unsigned long) <null> (bitcoind+0x132668) #1 ChainstateManager::InitializeChainstate(CTxMemPool*, std::__1::optional<uint256> const&) src/validation.cpp:4851:21 (bitcoind+0x48e26b) #2 node::LoadChainstate(bool, ChainstateManager&, CTxMemPool*, bool, Consensus::Params const&, bool, long, long, long, bool, bool, std::__1::function<bool ()>, std::__1::function<void ()>) src/node/chainstate.cpp:31:14 (bitcoind+0x24de07) #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1438:32 (bitcoind+0x14e994) #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) Mutex M131626 (0x7b3c00000898) created at: #0 pthread_mutex_lock <null> (bitcoind+0xda898) #1 std::__1::mutex::lock() <null> (libc++.so.1+0x49f35) #2 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74) #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e) #4 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e) #5 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e) #6 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e) #7 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e) #8 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f) #9 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f) #10 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f) #11 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a) #12 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a) #13 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a) Mutex M151 (0x55aacb8ea030) created at: #0 pthread_mutex_init <null> (bitcoind+0xbed2f) #1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3) #2 __libc_start_main <null> (libc.so.6+0x29eba) Mutex M131553 (0x7b4c000042e0) created at: #0 pthread_mutex_init <null> (bitcoind+0xbed2f) #1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3) #2 std::__1::__unique_if<CTxMemPool>::__unique_single std::__1::make_unique<CTxMemPool, CBlockPolicyEstimator*, int const&>(CBlockPolicyEstimator*&&, int const&) /usr/lib/llvm-13/bin/../include/c++/v1/__memory/unique_ptr.h:728:32 (bitcoind+0x15c81d) #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1426:24 (bitcoind+0x14e7b4) #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) Thread T22 'b-loadblk' (tid=32370, running) created by main thread at: #0 pthread_create <null> (bitcoind+0xbd5bd) #1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:443:10 (bitcoind+0x155e06) #2 std::__1::thread::thread<void (*)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, void>(void (*&&)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (bitcoind+0x155e06) #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1656:29 (bitcoind+0x150164) #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2) #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2) SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 in std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&) ================== ``` From https://cirrus-ci.com/task/5612886578954240?logs=ci#L4868 ACKs for top commit: achow101: re-ACK fac04cb theStack: Code-review ACK fac04cb Tree-SHA512: 9d619f99ff6373874c7ffe1db20674575605646b4b54b692fb54515a4a49f110a770026d7320ed6dfeaa7976be4cd89e93f821acdbf22c7662bd1c5be0cedcd2
DavidLacombe46
referenced
this pull request
in DavidLacombe46/bitcoin
Jan 5, 2023
tests: Add fuzzing harness for versionbits
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Non Master PR for collecting code review and feedback on OP_SECURETHEBAG implementation.