Skip to content

Conversation

lollerfirst
Copy link

@lollerfirst lollerfirst commented May 11, 2025

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.

@lollerfirst lollerfirst changed the base branch from master to generatetomany May 11, 2025 15:33
coinbaseTx.vout[i].scriptPubKey = scriptPubkey;

if (totalReward - amount < 0) {
throw std::runtime_error(strprintf("%s: Not enough block reward to satisfy coinbase outputs", __func__));
Copy link
Owner

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));

Copy link
Author

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

@polespinasa polespinasa left a 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.

@lollerfirst
Copy link
Author

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.

But also, due to fees being variable, it may be useful to have some kind of "remainder" option so that you can say, for example, "give 1 btc apiece to the first 3 addresses, and give the remainder to the last address"

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.

@polespinasa
Copy link
Owner

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.

@lollerfirst
Copy link
Author

@polespinasa I think this is closer to what you had envisioned.

@polespinasa polespinasa force-pushed the generatetomany branch 3 times, most recently from 9143274 to 5a5ff2a Compare May 12, 2025 09:16
@polespinasa
Copy link
Owner

I think this is closer to what you had envisioned.

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)

@lollerfirst lollerfirst force-pushed the generatetomany-address-split-remainder branch from 39028d7 to 551f5a2 Compare May 12, 2025 13:50
@lollerfirst
Copy link
Author

Rebased on your new "RPC centric" approach.

@polespinasa polespinasa force-pushed the generatetomany branch 3 times, most recently from cd8c13c to 8b5dd8a Compare May 20, 2025 09:57
@polespinasa
Copy link
Owner

@lollerfirst seems that we're moving it to generateblock instead of generatetomany.

Would you like to continue working on the fee collector part and rebase it or should I do it?

@lollerfirst
Copy link
Author

@polespinasa Yes will rebase.

@polespinasa polespinasa force-pushed the generatetomany branch 8 times, most recently from 4ff4e01 to 2c85569 Compare May 30, 2025 13:11
@polespinasa polespinasa force-pushed the generatetomany branch 4 times, most recently from f426866 to d89b900 Compare June 12, 2025 06:37
polespinasa pushed a commit that referenced this pull request Jun 26, 2025
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).
```
polespinasa pushed a commit that referenced this pull request Jun 26, 2025
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
@polespinasa
Copy link
Owner

Code should be rebased to work with the current status of master.

Functional tests still failing.

$ build/test/functional/rpc_generate.py 
2025-06-26T08:25:00.534643Z TestFramework (INFO): PRNG seed is: 1327152312913075691
2025-06-26T08:25:00.535194Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_dg7fiibk
2025-06-26T08:25:00.829342Z TestFramework (INFO): Test rpc generate raises with message to use cli option
2025-06-26T08:25:00.830560Z TestFramework (INFO): Test rpc generate help prints message to use cli option
2025-06-26T08:25:00.831481Z TestFramework (INFO): Test rpc generate is a hidden command not discoverable in general help
2025-06-26T08:25:00.877524Z TestFramework (INFO): Mine an empty block to address and return the hex
2025-06-26T08:25:00.887645Z TestFramework (INFO): Generate an empty block without address and fallback to OP_RETURN
2025-06-26T08:25:00.899228Z TestFramework (INFO): Generate an empty block to address
2025-06-26T08:25:00.908811Z TestFramework (INFO): Generate an empty block to a list of addresses
2025-06-26T08:25:00.915786Z TestFramework (ERROR): Key error
Traceback (most recent call last):
  File "/home/sliv3r/Documentos/Projectes/BitcoinCore/bitcoin/test/functional/test_framework/test_framework.py", line 190, in main
    self.run_test()
  File "/home/sliv3r/Documentos/Projectes/BitcoinCore/bitcoin/build/test/functional/rpc_generate.py", line 25, in run_test
    self.test_generateblock()
  File "/home/sliv3r/Documentos/Projectes/BitcoinCore/bitcoin/build/test/functional/rpc_generate.py", line 59, in test_generateblock
    assert_equal(block['tx'][0]['vout'][1]['scriptPubKey']['address'], address2)
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'address'
2025-06-26T08:25:00.967690Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
2025-06-26T08:25:00.967902Z TestFramework (WARNING): Not cleaning up dir /tmp/bitcoin_func_test_dg7fiibk
2025-06-26T08:25:00.967974Z TestFramework (ERROR): Test failed. Test logging available at /tmp/bitcoin_func_test_dg7fiibk/test_framework.log
2025-06-26T08:25:00.968100Z TestFramework (ERROR): 
2025-06-26T08:25:00.968263Z TestFramework (ERROR): Hint: Call /home/sliv3r/Documentos/Projectes/BitcoinCore/bitcoin/test/functional/combine_logs.py '/tmp/bitcoin_func_test_dg7fiibk' to consolidate all logs
2025-06-26T08:25:00.968322Z TestFramework (ERROR): 
2025-06-26T08:25:00.968371Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2025-06-26T08:25:00.968445Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2025-06-26T08:25:00.968499Z TestFramework (ERROR): 
[node 0] Cleaning up leftover process

To run all tests you can (after compiling):

  • Unit test: ctest --test-dir build
  • Functional test: build/test/functional/test_runner.py --extended
    When a functional test fails you can run each specific one with: build/test/functional/rpc_generate.py

@lollerfirst lollerfirst force-pushed the generatetomany-address-split-remainder branch 3 times, most recently from 22f4092 to a17b093 Compare July 7, 2025 08:48
@lollerfirst
Copy link
Author

@polespinasa Rebased. Changed from a std::map<std::string, CAmount> to std::vector<std::pair<std::string, CAmount>> so that the same address can be specified as multiple outputs (the reason why the test was failing).

@lollerfirst lollerfirst force-pushed the generatetomany-address-split-remainder branch from a17b093 to 9d30e01 Compare July 7, 2025 10:06
@polespinasa
Copy link
Owner

Hey! I will review when possible!

polespinasa pushed a commit that referenced this pull request Aug 18, 2025
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
polespinasa pushed a commit that referenced this pull request Aug 18, 2025
…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
polespinasa pushed a commit that referenced this pull request Aug 18, 2025
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).
```
polespinasa pushed a commit that referenced this pull request Aug 18, 2025
…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
@polespinasa polespinasa force-pushed the generatetomany branch 7 times, most recently from 912d9cb to 847fd39 Compare August 18, 2025 23:54
Copy link
Owner

@polespinasa polespinasa left a 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']
Copy link
Owner

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'))

Copy link
Author

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}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right 😅

@polespinasa polespinasa force-pushed the generatetomany branch 4 times, most recently from ae33a56 to 0d55bc4 Compare August 20, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants