-
Notifications
You must be signed in to change notification settings - Fork 1
GenerateToMany: custom outputs scripts split + equal split of the remainder #1
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
base: generatetomany
Are you sure you want to change the base?
GenerateToMany: custom outputs scripts split + equal split of the remainder #1
Conversation
src/node/miner.cpp
Outdated
coinbaseTx.vout[i].scriptPubKey = scriptPubkey; | ||
|
||
if (totalReward - amount < 0) { | ||
throw std::runtime_error(strprintf("%s: Not enough block reward to satisfy coinbase outputs", __func__)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a JSONRPCerror similar to:
throw JSONRPCError(RPC_INVALID_VALUE, strprintf("Invalid amount value for address at index %d", i));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but I was kinda reluctant to using an RPC error outside of an RPC module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check https://github.com/bitcoin/bitcoin/blob/59e09e0fb7b4cf8f8db97c2f81a8f6f69fe6cacf/src/wallet/rpc/spend.cpp#L176
A similar approach could be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not wrong this doesn't allow an address to be added with a fixed input + reminder. Should be focused that way imo.
This is the use case supertestnet referred to, and in this case you'd want to specify a fixed amount for some addresses, and then split the remaining block reward to other addresses if I am not missing anything. |
That's just the example he gives but a user could want to do something else. We shouldn't limit it this way. |
@polespinasa I think this is closer to what you had envisioned. |
9143274
to
5a5ff2a
Compare
Can you please squash commits so it's easier to check the code? Also, I think this will have to be rebased based on this review bitcoin#32468 (comment) |
39028d7
to
551f5a2
Compare
Rebased on your new "RPC centric" approach. |
cd8c13c
to
8b5dd8a
Compare
@lollerfirst seems that we're moving it to Would you like to continue working on the fee collector part and rebase it or should I do it? |
@polespinasa Yes will rebase. |
4ff4e01
to
2c85569
Compare
f426866
to
d89b900
Compare
Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and: ```bash export CC=clang export CXX=clang++ cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address cmake --build build export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan" ctest --test-dir build ``` ```bash Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms ********* Finished testing of AddressBookTests ********* ================================================================= ==21869==ERROR: LeakSanitizer: detected memory leaks Direct leak of 88 byte(s) in 1 object(s) allocated from: #0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) #1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18 bitcoin#4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5 bitcoin#5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75 bitcoin#6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13 bitcoin#7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5 bitcoin#8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5 bitcoin#9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) bitcoin#10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) bitcoin#11 0xffff8cf57c54 (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#12 0xffff8cf5fa18 (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#13 0xffff8cf6067c (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#14 0xffff8cf610a4 (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30 bitcoin#18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) bitcoin#19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) bitcoin#20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) ``` This happens when building using depends: ```bash Indirect leak of 24 byte(s) in 1 object(s) allocated from: #0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #1 0xfbff97f8c164 (<unknown module>) #2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o bitcoin#15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s). ```
cdd0826
to
f7a0929
Compare
5be31b2 lsan: add more Qt suppressions (fanquake) Pull request description: Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and: ```bash export CC=clang export CXX=clang++ cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address cmake --build build export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan" ctest --test-dir build ``` ```bash Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms ********* Finished testing of AddressBookTests ********* ================================================================= ==21869==ERROR: LeakSanitizer: detected memory leaks Direct leak of 88 byte(s) in 1 object(s) allocated from: #0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) #1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18 bitcoin#4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5 bitcoin#5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75 bitcoin#6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13 bitcoin#7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5 bitcoin#8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5 bitcoin#9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) bitcoin#10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) bitcoin#11 0xffff8cf57c54 (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#12 0xffff8cf5fa18 (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#13 0xffff8cf6067c (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#14 0xffff8cf610a4 (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) bitcoin#17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30 bitcoin#18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) bitcoin#19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) bitcoin#20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) ``` This happens when building using depends: ```bash Indirect leak of 24 byte(s) in 1 object(s) allocated from: #0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #1 0xfbff97f8c164 (<unknown module>) #2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) bitcoin#14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o bitcoin#15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s). ``` ACKs for top commit: maflcko: lgtm ACK 5be31b2 Tree-SHA512: 0c33661c7ec83ea9b874c1ee4ee2de513131690287363e216a88560dfb31a59ef563a50af756c86a991583aa64a600a74e20fd5d6a104cf4c0a27532de8d2211
Code should be rebased to work with the current status of master. Functional tests still failing.
To run all tests you can (after compiling):
|
22f4092
to
a17b093
Compare
@polespinasa Rebased. Changed from a |
a17b093
to
9d30e01
Compare
Hey! I will review when possible! |
67dc752 cmake, test: Disable tests instead of ignoring them (Hennadii Stepanov) bb9157d cmake, refactor: Switch to `Python3::Interpreter` imported target (Hennadii Stepanov) Pull request description: This PR: 1. Switches to a modern CMake approach by using the `Python3::Interpreter` imported target, which is more robust than using variables. 2. Disables the `util_rpcauth_test` test explicitly instead of silently ignoring it. A build and test log for the case when Python is unavailable is provided below: ``` $ cmake -B build $ cmake --build build -j 16 $ ctest --test-dir build -j $(nproc) -R "^util" Internal ctest changing into directory: /bitcoin/build Test project /bitcoin/build Start 115: util_tests Start 117: util_trace_tests Start 114: util_string_tests Start 116: util_threadnames_tests Start 1: util_rpcauth_test 1/5 Test #1: util_rpcauth_test ................***Not Run (Disabled) 0.00 sec 2/5 Test bitcoin#114: util_string_tests ................ Passed 0.11 sec 3/5 Test bitcoin#117: util_trace_tests ................. Passed 0.11 sec 4/5 Test bitcoin#116: util_threadnames_tests ........... Passed 0.11 sec 5/5 Test bitcoin#115: util_tests ....................... Passed 0.13 sec 100% tests passed, 0 tests failed out of 4 Total Test time (real) = 0.13 sec The following tests did not run: 1 - util_rpcauth_test (Disabled) ``` ACKs for top commit: purpleKarrot: ACK 67dc752 janb84: tACK 67dc752 Tree-SHA512: 5fc7ebe31ac03f4b8a53ecfcfc1cace0f647a1d2c989651988edae96bdfbbe2dee171714e57cb028e65ead1bb40806a82d9821746451dbf005538601fd33ea88
…xec in RunCommandJSON" faa1c3e Revert "Merge bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON" (MarcoFalke) Pull request description: After a fork() in a multithreaded program, the child can safely call only async-signal-safe functions (see [signal-safety(7)](https://www.man7.org/linux/man-pages/man7/signal-safety.7.html)) until such time as it calls execv. The standard library (`std` namespace) is not async-signal-safe. Also, `throw`, isn't. There was an alternative implementation using `readdir` (bitcoin#32529), but that isn't async-signal-safe either, and that implementation was still using `throw`. So temporarily revert this feature. A follow-up in the future can add it back, using only async-signal-safe functions, or by using a different approach. Fixes bitcoin#32524 Fixes bitcoin#33015 Fixes bitcoin#32855 For reference, a failure can manifest in the GCC debug mode: * While `fork`ing, a debug mode mutex is held (by any other thread). * The `fork`ed child tries to use the stdard libary before `execv` and deadlocks. This may look like the following: ``` (gdb) thread apply all bt Thread 1 (Thread 0xf58f4b40 (LWP 774911) "b-httpworker.2"): #0 0xf7f4f589 in __kernel_vsyscall () #1 0xf79e467e in ?? () from /lib32/libc.so.6 #2 0xf79eb582 in pthread_mutex_lock () from /lib32/libc.so.6 #3 0xf7d93bf2 in ?? () from /lib32/libstdc++.so.6 bitcoin#4 0xf7d93f36 in __gnu_debug::_Safe_iterator_base::_M_attach(__gnu_debug::_Safe_sequence_base*, bool) () from /lib32/libstdc++.so.6 bitcoin#5 0x5668810a in __gnu_debug::_Safe_iterator_base::_Safe_iterator_base (this=0xf58f13ac, __seq=0xf58f13f8, __constant=false) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_base.h:91 bitcoin#6 0x56ddfb50 in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::forward_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:162 bitcoin#7 0x56ddfacb in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::bidirectional_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:539 bitcoin#8 0x56ddfa5b in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::random_access_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:687 bitcoin#9 0x56ddd3f6 in std::__debug::vector<int, std::allocator<int> >::begin (this=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/vector:300 bitcoin#10 0x57d83701 in subprocess::detail::Child::execute_child (this=0xf58f156c) at ./util/subprocess.h:1372 bitcoin#11 0x57d80a7c in subprocess::Popen::execute_process (this=0xf58f1cd8) at ./util/subprocess.h:1231 bitcoin#12 0x57d6d2b4 in subprocess::Popen::Popen<subprocess::input, subprocess::output, subprocess::error, subprocess::close_fds> (this=0xf58f1cd8, cmd_args="fake.py enumerate", args=..., args=..., args=..., args=...) at ./util/subprocess.h:964 bitcoin#13 0x57d6b597 in RunCommandParseJSON (str_command="fake.py enumerate", str_std_in="") at ./common/run_command.cpp:27 bitcoin#14 0x57a90547 in ExternalSigner::Enumerate (command="fake.py", signers=std::__debug::vector of length 0, capacity 0, chain="regtest") at ./external_signer.cpp:28 bitcoin#15 0x56defdab in enumeratesigners()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const (this=0xf58f2ba0, self=..., request=...) at ./rpc/external_signer.cpp:51 ... (truncated, only one thread exists) ``` ACKs for top commit: fanquake: ACK faa1c3e darosior: ACK faa1c3e Tree-SHA512: 602da5f2eba08d7fe01ba19baf411e287ae27fe2d4b82f41734e05b7b1d938ce94cc0041e86ba677284fa92838e96ebee687023ff28047e2b036fd9a53567e0a
The `-assumevalid` option skips script verification for a specified block and all its ancestors during Initial Block Download. Many new users are surprised when this suddenly slows their node to a halt. This commit adds a log message to clearly indicate when this optimization ends and full validation begins (and vice versa). When using `-assumeutxo`, logging is suppressed for the active assumed-valid chainstate and for the background validation chainstate to avoid the confusing toggles. ------- > cmake -B build && cmake --build build && mkdir -p demo && build/bin/bitcoind -datadir=demo -stopatheight=500 | grep 'signature validation' ``` 2025-08-08T20:59:21Z Disabling signature validations at block #1 (00000000839a8e6886ab5951d76f411475428afc90947ee320161bbf18eb6048). 2025-08-08T20:59:21Z Enabling signature validations at block bitcoin#100 (000000007bc154e0fa7ea32218a72fe2c1bb9f86cf8c9ebf9a715ed27fdb229a). 2025-08-08T20:59:21Z Disabling signature validations at block bitcoin#200 (000000008f1a7008320c16b8402b7f11e82951f44ca2663caf6860ab2eeef320). 2025-08-08T20:59:21Z Enabling signature validations at block bitcoin#300 (0000000062b69e4a2c3312a5782d7798b0711e9ebac065cd5d19f946439f8609). ```
…hange fab2980 assumevalid: log every script validation state change (Lőrinc) Pull request description: The `-assumevalid` option skips script verification for a specified block and all its ancestors during Initial Block Download. Many new [users are surprised](bitcoin#32832) when this suddenly slows their node to a halt. This commit adds a log message to clearly indicate when this optimization ends and full validation begins (and vice versa). <details> <summary>Testing instructions</summary> The behavior can easily be tested by adding this before the new log: ```C++ // TODO hack to enable/disable script checks based on block height for testing purposes if (pindex->nHeight < 100) fScriptChecks = false; else if (pindex->nHeight < 200) fScriptChecks = true; else if (pindex->nHeight < 300) fScriptChecks = false; else if (pindex->nHeight < 400) fScriptChecks = true; ``` and exercise the new code with: ```bash cmake -B build && cmake --build build && mkdir -p demo && build/bin/bitcoind -datadir=demo -stopatheight=500 | grep 'signature validation' ``` showing something like: * Disabling signature validations at block #1 (00000000839a8e6886ab5951d76f411475428afc90947ee320161bbf18eb6048). * Enabling signature validations at block bitcoin#100 (000000007bc154e0fa7ea32218a72fe2c1bb9f86cf8c9ebf9a715ed27fdb229a). * Disabling signature validations at block bitcoin#200 (000000008f1a7008320c16b8402b7f11e82951f44ca2663caf6860ab2eeef320). * Enabling signature validations at block bitcoin#300 (0000000062b69e4a2c3312a5782d7798b0711e9ebac065cd5d19f946439f8609). </details> ACKs for top commit: achow101: ACK fab2980 ajtowns: crACK fab2980 davidgumberg: untested crACK bitcoin@fab2980 Tree-SHA512: e90b66f7423b639356daace476942ce83e65e70466544394cbe2f15738bdbf716163eaf590c64c5448f9b41aeeaafe3342c48c6a7a478678a70b0310ca94e11d
912d9cb
to
847fd39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I couldn't review earlier :(
This has to be rebased again, there were merge conflicts with Core Master Branch.
The rebase of the original PR bitcoin#32468 broke one test for some reason I don't understand. Checking it is appreciated :)
self.log.info('Generate an empty block to a list of addresses and custom reward split') | ||
address2 = miniwallet.get_address() | ||
address3 = miniwallet.get_address() | ||
hash = self.generateblock(node, outputs=json.dumps([{address: 100000000}, {address2: 0}, {address3: 0}]), transactions=[])['hash'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really follow the behavior of this. We set 1BTC to address 1. So if the total reward is 25 it will be splited like 9+8+8 right?
If I fix 4BTC for address 1. Then the total reward should be 11+7+7?
If so there's something wrong:
AssertionError: not(8.36000000 == 11.00000000)
hash = self.generateblock(node, outputs=json.dumps([{address: 4000000}, {address2: 0}, {address3: 0}]), transactions=[])['hash']
block = node.getblock(blockhash=hash, verbose=2)
assert_equal(len(block['tx']), 1)
assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['address'], address)
assert_equal(block['tx'][0]['vout'][1]['scriptPubKey']['address'], address2)
assert_equal(block['tx'][0]['vout'][2]['scriptPubKey']['address'], address3)
assert_equal(block['tx'][0]['vout'][0]['value'], Decimal('11.00000000'))
assert_equal(block['tx'][0]['vout'][1]['value'], Decimal('7.00000000'))
assert_equal(block['tx'][0]['vout'][2]['value'], Decimal('7.00000000'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{address: 4000000}
aren't you missing a couple of zeros here? {address: 400000000}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right 😅
ae33a56
to
0d55bc4
Compare
Can assign a custom amount split to every output or "remainder" to indicate that said output should be assigned an equal split of whatever is left.
In the case the custom splits exceed the block reward, throw a runtime error.