Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update from bitcoin v24.1 to v25.1 #16

Open
wants to merge 1,971 commits into
base: itcoin
Choose a base branch
from
Open

Conversation

muxator
Copy link

@muxator muxator commented Oct 11, 2023

Bitcoin v25.1rc1 was released on 2023-10-04 (https://github.com/bitcoin/bitcoin/tree/v25.1rc1).

We can safely assume that the churn upstream from rc1 to the final v25.1 release will not be very noisy, so it is time to start the preparatory work to upgrade the itcoin-core base from bitcoin v24.1 to bitcoin v25.1.

Known issues:

  1. does not compile

    • blockencodings.cpp: In member function ‘ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock&, const std::vector<std::shared_ptr<const CTransaction> >&)’:
      blockencodings.cpp:205:51: error: operands to ‘?:’ have different types ‘PartiallyDownloadedBlock::CheckBlockFn’ {aka ‘std::function<bool(const CBlock&, BlockValidationState&, const Consensus::Params&, bool, bool)>’} and ‘bool(const CBlock&, BlockValidationState&, const Consensus::Params&, bool, bool, bool)’
        205 |     CheckBlockFn check_block = m_check_block_mock ? m_check_block_mock : CheckBlock;
            |                                ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
    • In file included from /usr/include/boost/test/test_tools.hpp:45,
                       from /usr/include/boost/test/unit_test.hpp:18,
                       from wallet/test/wallet_tests.cpp:28:
      wallet/test/wallet_tests.cpp: In member function ‘void wallet::wallet_tests::ListCoinsTest::test_method()’:
      wallet/test/wallet_tests.cpp:584:36: error: ‘GetAvailableBalance’ was not declared in this scope
        584 |     BOOST_CHECK_EQUAL(5050 * COIN, GetAvailableBalance(*wallet)); // ITCOIN_SPECIFIC: it was 50, but COINBASE_MATURITY=0 implies that in a 101 blocks chain we have 5050 * COIN balance
            |                                    ^~~~~~~~~~~~~~~~~~~
      
  2. cant' run the tests since it does not compile yet

  3. Upgrading https://github.com/bancaditalia/secp256k1-frost and vendorizing it was not handled at all yet

  4. the local initialization via:

    cd infra
    ./initialize-itcoin-local.sh
    ./continue-mining-local.sh
    

    is not working.

5: files that between v24.1-itcoin-1 e v25.1rc1 were deleted, but that contained itcoin-specific patches that have to be reapplied somewhere else (we have information loss here):

  • src/wallet/test/availablecoins_tests.cpp
  • test/functional/rpc_fundrawtransaction.py

TODO: find where those files have been moved to, and integrate back itcoin modifications

  1. little modifications that we should heve done in v24.1-itcoin-1 already:
    • test/functional/wallet_signrawtransactionwithwallet.py
      in test_fully_signed_tx() change:

      self.generate(self.nodes[0], 100 + 1) # ITCOIN_SPECIFIC: it was COINBASE_MATURITY + 100 but at least 100 blocks are required in test_signing_with_cltv.
      

      to:

      self.generate(self.nodes[0], 100 + 1) # ITCOIN_SPECIFIC: it was COINBASE_MATURITY + 1 but at least 100 blocks are required in test_signing_with_cltv.
      
    • in test/functional/test_framework/test_node.py change:

      from test_framework.conftest import ITCOIN_PBFT_ROOT_DIR
      

      to:

      from test_framework.conftest import ITCOIN_PBFT_ROOT_DIR  # ITCOIN_SPECIFIC
      

fanquake and others added 30 commits March 29, 2023 12:09
4133c81 guix: use gcc tool wrappers (fanquake)

Pull request description:

  This way, correct `--plugin` arguments are passed through.

  This is a prerequisite for LTO (see bitcoin#25391). Split out, to try move things along, as this change is isolated, and should be straight-forward.

ACKs for top commit:
  TheCharlatan:
    ACK [4133c81](bitcoin@4133c81)
  hebasto:
    ACK 4133c81

Tree-SHA512: 4311a72a613cf027bd4490caa29604c985ed455589acd972285f13cbdf4806d2184a4dc6f20cb6f47c3fa751d58bfd0bacc257b87d4a804bf5ecf5b240e4a757
This further minifies the Guix release build environment.
…tances

ea7ec78 refactor: Drop no longer used `CNetMsgMaker` instances (Hennadii Stepanov)

Pull request description:

  The removed lines have been unused since the bitcoin@abf5d16 commit from bitcoin#25454.

ACKs for top commit:
  dergoegge:
    utACK ea7ec78
  Sjors:
    ACK ea7ec78
  TheCharlatan:
    ACK ea7ec78

Tree-SHA512: 9a2a9ff3f124b68a8cd20a637e90885096996c3aa354a4d8adbec98f5761e9e826c1c064ccd90aaf6d72beac61dd9e22c8b76d089e18bba6e0ad51e59a9c7df8
a634c28 ci: use LLVM/clang-16 in native_fuzz (ASAN) job (fanquake)

Pull request description:

  Similar to bitcoin#27298.

ACKs for top commit:
  dergoegge:
    utACK a634c28

Tree-SHA512: 7a2625a3ac83710063d941dcbca42431b3b79a1380872fd2c566c0ab3041d8123d7dcddeb8a4972efd0ef6496b15bbe0b39b6d2de84df81fcdd8d68e1248fbc5
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); }

ren nUnconnectingHeaders     m_num_unconnecting_headers_msgs
ren fPreferHeaders           m_prefers_headers
ren MAX_UNCONNECTING_HEADERS MAX_NUM_UNCONNECTING_HEADERS_MSGS

-END VERIFY SCRIPT-
I am no-longer seeing this. Can anyone recreate the false-positive?
…sgproc_mutex to Peer

3a060ae scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders (dergoegge)
279c53d [net processing] Move m_recently_announced_invs from CNodeState to Peer (dergoegge)
938a8e2 [net processing] Annotate m_recently_announced_invs as guarded by g_msgproc_mutex (dergoegge)
8a2cb1f [net processing] Move fPreferHeaders from CNodeState to Peer (dergoegge)
3605011 [net processing] Annotate fPreferHeaders as guarded by g_msgproc_mutex (dergoegge)
4b84e50 [net processing] Move m_headers_sync_timeout from CNodeState to Peer (dergoegge)
689b747 [net processing] Annotate m_headers_sync_timeout as guarded by g_msgproc_mutex (dergoegge)
d8c0d1c [net processing] Move nUnconnectingHeaders from CNodeState to Peer (dergoegge)
5f80d8d [net processing] Annotate nUnconnectingHeaders as guarded by g_msgproc_mutex (dergoegge)
1d87137 [validation] Annotate ChainstateManager::m_best_header as guarded by cs_main (dergoegge)

Pull request description:

  `nUnconnectingHeaders`, `m_headers_sync_timeout`, `fPreferHeaders` and  `m_recently_announced_headers` are currently all `CNodeState` members even though they are only ever accessed from the message processing thread (therefore sufficiently guarded exclusively by `g_msgproc_mutex`). `CNodeState` exists purely to hold validation-specific state guarded by `cs_main` that is accessed by multiple threads.

  This PR adds thread-safety annotations for the above mentioned `CNodeState` members and moves them to `Peer`.

ACKs for top commit:
  glozow:
    code review ACK 3a060ae, as in I am convinced these members shouldn't be guarded by cs_main and belong in Peer/TxRelay. clang checked the annotations for me.
  hebasto:
    ACK 3a060ae

Tree-SHA512: 2db27c03f2c6ed36ad7dfbb4f862eeed3c3e57f845cf8abb9e7cada36f976257311892020bbcff513fbe662a881c93270e3a126946ceb0c3f94213b546bcaa81
…subtests via decorator

e669833 test: dedup package limit checks via decorator in mempool_package_limits.py (Sebastian Falbesoner)
72f25e2 test: refactor: use Satoshis for fees in mempool_package_limits.py (Sebastian Falbesoner)

Pull request description:

  The subtests in the functional test mempool_package_limits.py all follow the same pattern:
  1. first, check that the mempool is currently empty
  2. create and submit certain single txs to the mempool, prepare list of hex transactions
  3. check that `testmempoolaccept` on the package hex fails with a "package-mempool-limits" error on each tx result
  4. after mining a block, check that submitting the package succeeds

  Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so this might be a nice opportunity to deduplicate code by using a newly introduced decorator which executes the necessary before and after the essential part of the subtest. This also makes it easier to add new subtests without having to copy-paste those parts once again.

  In addition, the first commit switches the fee unit from BTC to Satoshis, which allows to get rid of some imports (`COIN` and `Decimal`) and a comment for the `test_desc_size_limits` subtest is fixed (s/25KvB/21KvB/).

ACKs for top commit:
  ismaelsadeeq:
    ACK e669833
  glozow:
    utACK e669833

Tree-SHA512: 84a85e739de7387391c13bd46aeb015a74302ea7c6f0ca3d4e2b1b487d38df390dc118eb5b1c11d3e4206bff316a4dab60ef6b25d8feced672345d4e36ffd205
b5ef141 ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs bitcoin#27321) (Vasil Stoyanov)

Pull request description:

  Basically it removes the above-mentioned env-vars as per MarcoFalke's instructions. The only deviation from the plan laid out there was that I double-quoted the last instance of $ANDROID_HOME for the sake of consistency and future-proofing and the rest of the non-quoted vars due to lint failing the build.

  Fixes bitcoin#27321.

ACKs for top commit:
  josibake:
    ACK bitcoin@b5ef141
  hernanmarino:
    untested ACK b5ef141. LGTM

Tree-SHA512: a79776bf64a2fa8b38195cc84445e171fd689f156aac5a1e5d39040300567eb9f4c2ebd00fbf3fa0e55b68793f8f752d94f7d817f6097ed9dd3a8ea57651b981
d0e571e guix: use python-minimal (3.9) (fanquake)

Pull request description:

  This further minifies the Guix release build environment.

ACKs for top commit:
  TheCharlatan:
    ACK d0e571e
  hebasto:
    ACK d0e571e

Tree-SHA512: 0a8aa9ae861107f106c3b9c41f78ffbaf0e71e3c61f6d96e5c82415b4570b8ac85d6578d37cd0df0ec315c1c9f35fc90b281f139271ccfd15a1495ba76166789
71b3e9b sanitizers: remove GetRNGState lsan suppression (fanquake)

Pull request description:

  I am no-longer seeing this, testing with the native_asan job over `x86_64` (Ubuntu 22.04) and `aarch64` (Fedora 37).

  Can anyone recreate the false-positive?

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 71b3e9b
  hebasto:
    ACK 71b3e9b, tested on Ubuntu 22.04 x86_64.

Tree-SHA512: 63020327d61acd6c94c6c278c9c4d72aedc10253fa172bcf9353bcad4c28d068bee824969eb3ce92152244831df8fe92cffae536453c8073a4fda74dfdfbcefa
Taking cs_main is no longer necessary since we moved
`m_recently_announced_invs` to `Peer` and `mapRelay` is actually only
accessed from the message processing thread.
stickies-v and others added 9 commits October 4, 2023 10:11
There is no significant benefit in logging the request count instead
of the connection count. Reduces amount of code and computational
complexity.

Github-Pull: bitcoin#28551
Rebased-From: 084d037
It is possible that the client disconnects before the request is
handled. In those cases, evhttp_request_set_on_complete_cb is never
called, which means that on shutdown the server we'll keep waiting
endlessly.

By adding evhttp_connection_set_closecb, libevent automatically
cleans up those dead connections at latest when we shutdown, and
depending on the libevent version already at the moment of remote
client disconnect. In both cases, the bug is fixed.

Github-Pull: bitcoin#28551
Rebased-From: 68f23f5
45a5fcb http: bugfix: track closed connection (stickies-v)
752a456 http: log connection instead of request count (stickies-v)
ae86ada http: refactor: use encapsulated HTTPRequestTracker (stickies-v)
f31899d gui: macOS, make appMenuBar part of the main app window (furszy)
64ffa94 gui: macOS, do not process dock icon actions during shutdown (furszy)
e270f3f depends: fix unusable memory_resource in macos qt build (fanquake)
a668394 build, macos: Fix `qt` package build with new Xcode 15 linker (Hennadii Stepanov)
b3517cb test: Test loading wallets with conflicts without a chain (Andrew Chow)
d63478c wallet: Check last block and conflict height are valid in MarkConflicted (Andrew Chow)
5e51a9c ci: Nuke Android APK task, Use credits for tsan (MarcoFalke)
910c362 test: ensure old fee_estimate.dat not read on restart and flushed (ismaelsadeeq)
37764d3 tx fees, policy: read stale fee estimates with a regtest-only option (ismaelsadeeq)
16bb916 tx fees, policy: do not read estimates of old fee_estimates.dat (ismaelsadeeq)
c4dd598 tx fees, policy: periodically flush fee estimates to fee_estimates.dat (ismaelsadeeq)
c36770c test: wallet, verify migration doesn't crash for an invalid script (furszy)
0d2a33e wallet: disallow migration of invalid or not-watched scripts (furszy)
2c51a07 Do not use std::vector = {} to release memory (Pieter Wuille)

Pull request description:

  Further backports for the `25.x` branch. Currently:
  * bitcoin#27622
  * bitcoin#27834
  * bitcoin#28125
  * bitcoin#28452
  * bitcoin#28542
  * bitcoin#28543
  * bitcoin#28551
  * bitcoin#28571
  * bitcoin-core/gui#751

ACKs for top commit:
  hebasto:
    re-ACK 45a5fcb, only bitcoin#28551 has been backported with since my recent [review](bitcoin#28487 (review)).
  dergoegge:
    reACK 45a5fcb
  willcl-ark:
    reACK 45a5fcb

Tree-SHA512: 0f5807aa364b7c2a2039fef11d5cd5e168372c3bf5b0e941350fcd92e7db4a1662801b97bb4f68e29788c77d24bbf97385a483c4501ca72d93fa25327d5694fa
10f3f81 doc: add release notes for 25.1rc1 (fanquake)
71aed7a doc: update manual pages for 25.1rc1 (fanquake)
02f059c build: Bump version to 25.1rc1 (fanquake)
dc1fcec doc: add 25.0 release notes (fanquake)

Pull request description:

  Final changes to tag a `25.1rc1`.
  Bumps version numbers, man pages, adds release notes etc.

ACKs for top commit:
  dergoegge:
    ACK 10f3f81
  hebasto:
    ACK 10f3f81, I have reviewed the code and it looks OK.
  willcl-ark:
    ACK 10f3f81
  stickies-v:
    ACK 10f3f81

Tree-SHA512: 3e1c527dac06afb4c8e1e481055211f69aa0ed769ac242ed610d23d470bbc0c062227034d01876ba4a5fa836e953fb001e8b0d6bc026069d372d34b8a031fb25
@muxator
Copy link
Author

muxator commented Oct 11, 2023

v2:

  • the build is now working:

    $ bin/bitcoind --version
    Itcoin Core version v25.1.0rc1-itcoin-1
    Copyright (C) 2019-2023 The Bank of Italy developers
    Copyright (C) 2009-2023 The Bitcoin Core developers
    
  • make check fails. To reproduce the problems, run: src/test/test_bitcoin --log_level=all --run_test=wallet_tests.
    Current situation:

    1. wallet/test/wallet_tests.cpp(662): error: in "wallet_tests/BasicOutputTypesTest": check available_coins.Size() == expected_coins_sizes[OutputType::UNKNOWN] has failed [101 != 1]
      
    2. wallet/test/wallet_tests.cpp(663): error: in "wallet_tests/BasicOutputTypesTest": check available_coins.coins[OutputType::UNKNOWN].size() == expected_coins_sizes[OutputType::UNKNOWN] has failed [101 != 1]
      2023-10-11T14:33:51.286312Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] Fee Calculation: Fee:0 Bytes:191 Tgt:0 (requested 6) Reason:"Fallback fee" Decay 0.00000: Estimation: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out) Fail: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out)
      2023-10-11T14:33:51.286376Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] CommitTransaction:
      CTransaction(hash=2154169602, ver=2, vin.size=1, vout.size=2, nLockTime=101)
          CTxIn(COutPoint(111704014a, 0), scriptSig=47304402203d97a75e025ddd, nSequence=4294967293)
          CScriptWitness()
          CTxOut(nValue=1.00000000, scriptPubKey=76a914d875dd2e0ed88047f02a1afc)
          CTxOut(nValue=49.00000000, scriptPubKey=76a9147d5925e4e65bc40e16881723)
      
    3. wallet/test/wallet_tests.cpp(650): error: in "wallet_tests/BasicOutputTypesTest": check size == available_coins.coins[type].size() has failed [1 != 100]
      2023-10-11T14:33:51.317924Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] Fee Calculation: Fee:0 Bytes:187 Tgt:0 (requested 6) Reason:"Fallback fee" Decay 0.00000: Estimation: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out) Fail: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out)
      2023-10-11T14:33:51.317986Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] CommitTransaction:
      CTransaction(hash=36aab314f2, ver=2, vin.size=1, vout.size=2, nLockTime=102)
          CTxIn(COutPoint(704f71c5ac, 0), scriptSig=47304402201d705be06ca00b, nSequence=4294967293)
          CScriptWitness()
          CTxOut(nValue=1.00000000, scriptPubKey=a9147fd8cd441ab0c66f46a8136f1b)
          CTxOut(nValue=49.00000000, scriptPubKey=a914958c38db29b055810b3ab5581c)
      
    4. wallet/test/wallet_tests.cpp(650): error: in "wallet_tests/BasicOutputTypesTest": check size == available_coins.coins[type].size() has failed [1 != 99]
      2023-10-11T14:33:51.348989Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] Fee Calculation: Fee:0 Bytes:185 Tgt:0 (requested 6) Reason:"Fallback fee" Decay 0.00000: Estimation: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out) Fail: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out)
      2023-10-11T14:33:51.349052Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] CommitTransaction:
      CTransaction(hash=58f384e078, ver=2, vin.size=1, vout.size=2, nLockTime=103)
          CTxIn(COutPoint(424a7d4eec, 0), scriptSig=4730440220270f65a5ad4a7e, nSequence=4294967293)
          CScriptWitness()
          CTxOut(nValue=49.00000000, scriptPubKey=00140a85442bc6be40176f21c5895d)
          CTxOut(nValue=1.00000000, scriptPubKey=001404164ae2d10ad0615127eab205)
      
    5. wallet/test/wallet_tests.cpp(650): error: in "wallet_tests/BasicOutputTypesTest": check size == available_coins.coins[type].size() has failed [1 != 98]
      2023-10-11T14:33:51.380354Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] Fee Calculation: Fee:0 Bytes:209 Tgt:0 (requested 6) Reason:"Fallback fee" Decay 0.00000: Estimation: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out) Fail: (-1 - -1) 0.00% 0.0/(0.0 0 mem 0.0 out)
      2023-10-11T14:33:51.380417Z (mocktime: 2020-08-31T15:34:12Z) [test] [wallet/wallet.h:869] [WalletLogPrintf] [default wallet] CommitTransaction:
      CTransaction(hash=6d9e72ebba, ver=2, vin.size=1, vout.size=2, nLockTime=104)
          CTxIn(COutPoint(a929eb7f80, 0), scriptSig=4730440220599c6201f54180, nSequence=4294967293)
          CScriptWitness()
          CTxOut(nValue=1.00000000, scriptPubKey=5120fa1d480d1ade439f29e837a48f)
          CTxOut(nValue=49.00000000, scriptPubKey=512094cf7ff4a2f9f35fef029e27df)
      
    6. wallet/test/wallet_tests.cpp(650): error: in "wallet_tests/BasicOutputTypesTest": check size == available_coins.coins[type].size() has failed [1 != 97]
      
    *** 6 failures are detected in the test module "Bitcoin Core Test Suite"
    
  • the following functional tests are failing:

    feature_signet.py                                        | ✖ Failed  | 3 s
    interface_zmq.py                                         | ✖ Failed  | 1 s
    itcoin_feature_control_block_time.py                     | ✖ Failed  | 1 s
    itcoin_feature_num_nodes_differs_from_num_signers.py     | ✖ Failed  | 1 s
    itcoin_feature_solution_independent_blockchain_1_of_2.py | ✖ Failed  | 1 s
    itcoin_feature_solution_independent_blockchain_3_of_4.py | ✖ Failed  | 1 s
    itcoin_feature_taproot_challenge_frost_pubkey.py         | ✖ Failed  | 4 s
    itcoin_feature_taproot_challenge_frost_pubkey_tweaked.py | ✖ Failed  | 5 s
    itcoin_rpc_testblockvalidity_1.py                        | ✖ Failed  | 1 s
    itcoin_rpc_testblockvalidity_2.py                        | ✖ Failed  | 1 s
    itcoin_zmq_itcoinblock.py                                | ✖ Failed  | 1 s
    rpc_getblockfrompeer.py                                  | ✖ Failed  | 7 s
    rpc_rawtransaction.py --legacy-wallet                    | ✖ Failed  | 9 s
    rpc_scantxoutset.py                                      | ✖ Failed  | 1 s
    rpc_txoutproof.py                                        | ✖ Failed  | 1 s
    wallet_balance.py --descriptors                          | ✖ Failed  | 3 s
    wallet_balance.py --legacy-wallet                        | ✖ Failed  | 3 s
    wallet_fundrawtransaction.py --descriptors               | ✖ Failed  | 20 s
    wallet_fundrawtransaction.py --legacy-wallet             | ✖ Failed  | 14 s
    wallet_transactiontime_rescan.py --legacy-wallet         | ✖ Failed  | 20 s
    

…chainparams.cpp the modifications that originally where in src/chainparams.cpp

Now the commands:
    cd infra
    ./initialize-itcoin-local.sh
    ./continue-mining-local.sh

Run instantly because the PoW is minimized.
@muxator
Copy link
Author

muxator commented Oct 11, 2023

v3:

  • ./initialize-itcoin-local.sh && ./continue-mining-local.sh is now working
  • reintegrated the removal of the PoW, applying back our changes in src/kernel/chainparams.cpp (they were moved there from src/chainparams.cpp.
  • for this reason, ./continue-mining-local.sh now runs instantly.

muxator and others added 8 commits October 11, 2023 23:28
…on.py in fa7d71a

The test is still not completely working.
…ECIFIC modifications that were in src/wallet/test/availablecoins_tests.cpp

src/wallet/test/availablecoins_tests.cpp was present in v24.1, but was removed
in v25.1, and its content integrated in wallet_tests.cpp. Let's bring there our
ITCOIN_SPECIFIC modifications. We lost them when doing the textual merge at the
start of this series.
a136700 doc: update release notes for 25.1 (fanquake)
e8d5c35 doc: update manual pages for 25.1 (fanquake)
9e00b73 build: bump version to 25.1 final (fanquake)

Pull request description:

  Final changes for `v25.1`.
  PR for bitcoincore.org is here: bitcoin-core/bitcoincore.org#991.
  No additional changes have been backported since rc1 (tagged 14 days ago),
  and no bugs or issues have been reported (test bins have been available for 9 days).

ACKs for top commit:
  hebasto:
    ACK a136700.
  TheCharlatan:
    lgtm ACK a136700

Tree-SHA512: 037f937dd4d2a9de15ff15a80f35b6047b61ffc3d9a33e7d4a4fcbce6302569bbc516c6da5849211b34ebbe914c4edcdd182e2d1d43897d0dcd32834ce6f2574
Just a preliminary merge.

Knwn issues:
1. does not compile
   - blockencodings.cpp: In member function ‘ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock&, const std::vector<std::shared_ptr<const CTransaction> >&)’:
     blockencodings.cpp:205:51: error: operands to ‘?:’ have different types ‘PartiallyDownloadedBlock::CheckBlockFn’ {aka ‘std::function<bool(const CBlock&, BlockValidationState&, const Consensus::Params&, bool, bool)>’} and ‘bool(const CBlock&, BlockValidationState&, const Consensus::Params&, bool, bool, bool)’
       205 |     CheckBlockFn check_block = m_check_block_mock ? m_check_block_mock : CheckBlock;
           |                                ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

2. cant' run the tests since it does not compile yet

3. importing and modification of secp256k1 was not handled at all

4: files that between v24.1-itcoin e v25.1rc1 were deleted, but that contained
   itcoin-specific patches that have to be reapplied somewhere else:
   - src/wallet/test/availablecoins_tests.cpp
   - test/functional/rpc_fundrawtransaction.py

   TODO: find where those files have been moved to, and integrate back itcoin modifications

5. little modifications that we should heve done in v24.1-itcoin-1 already:
   - test/functional/wallet_signrawtransactionwithwallet.py
     in test_fully_signed_tx() change:
         self.generate(self.nodes[0], 100 + 1) # ITCOIN_SPECIFIC: it was COINBASE_MATURITY + 100 but at least 100 blocks are required in test_signing_with_cltv.
     to:
         self.generate(self.nodes[0], 100 + 1) # ITCOIN_SPECIFIC: it was COINBASE_MATURITY + 1 but at least 100 blocks are required in test_signing_with_cltv.

    - test/functional/test_framework/test_node.py
      change:
          from test_framework.conftest import ITCOIN_PBFT_ROOT_DIR
      to:
          from test_framework.conftest import ITCOIN_PBFT_ROOT_DIR  # ITCOIN_SPECIFIC
@muxator muxator force-pushed the itcoin-migrate-to-v25.1-rc1 branch from 2ee48ec to 7710cd7 Compare October 24, 2023 09:36
@muxator
Copy link
Author

muxator commented Oct 24, 2023

v3:

  • rebased on bitcoin v25.1 final, released on 2023-10-19

@muxator muxator force-pushed the itcoin-migrate-to-v25.1-rc1 branch from 7710cd7 to a235814 Compare December 4, 2023 10:00
muxator pushed a commit that referenced this pull request Dec 4, 2023
…tifications fuzz target

fab164f fuzz: Avoid signed-integer-overflow in wallet_notifications fuzz target (MarcoFalke)

Pull request description:

  Should avoid

  ```
  policy/feerate.cpp:29:63: runtime error: signed integer overflow: 77600710321911316 * 149 cannot be represented in type 'int64_t' (aka 'long')
      #0 0x563a1775ed66 in CFeeRate::GetFee(unsigned int) const src/policy/feerate.cpp:29:63
      #1 0x563a15913a69 in wallet::COutput::COutput(COutPoint const&, CTxOut const&, int, int, bool, bool, bool, long, bool, std::optional<CFeeRate>) src/./wallet/coinselection.h:91:57
      #2 0x563a16fa6a6d in wallet::FetchSelectedInputs(wallet::CWallet const&, wallet::CCoinControl const&, wallet::CoinSelectionParams const&) src/wallet/spend.cpp:297:17
      #3 0x563a16fc4512 in wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, int, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1105:33
      #4 0x563a16fbec74 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, int, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1291:16
      #5 0x563a16fcf6df in wallet::FundTransaction(wallet::CWallet&, CMutableTransaction&, long&, int&, bilingual_str&, bool, std::set<int, std::less<int>, std::allocator<int>> const&, wallet::CCoinControl) src/wallet/spend.cpp:1361:16
      #6 0x563a1597b7b9 in wallet::(anonymous namespace)::FuzzedWallet::FundTx(FuzzedDataProvider&, CMutableTransaction) src/wallet/test/fuzz/notifications.cpp:162:15
      #7 0x563a15958240 in wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0::operator()() const src/wallet/test/fuzz/notifications.cpp:228:23
      #8 0x563a15958240 in unsigned long CallOneOf<wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_1>(FuzzedDataProvider&, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_1) src/./test/fuzz/util.h:43:27
      #9 0x563a15958240 in wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>) src/wallet/test/fuzz/notifications.cpp:196:9
      #10 0x563a15fdef0c in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
      #11 0x563a15fdef0c in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:178:5
      #12 0x563a158032a4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19822a4) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #13 0x563a15802999 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1981999) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #14 0x563a15804586 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1983586) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #15 0x563a15804aa7 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1983aa7) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #16 0x563a157f21fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19711fb) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #17 0x563a1581c766 in main (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x199b766) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #18 0x7f499e17b0cf  (/lib/x86_64-linux-gnu/libc.so.6+0x280cf) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
      bitcoin#19 0x7f499e17b188 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28188) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
      bitcoin#20 0x563a157e70c4 in _start (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19660c4) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)

  SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow policy/feerate.cpp:29:63 in
  MS: 0 ; base unit: 0000000000000000000000000000000000000000
  0x3f,0x0,0x2f,0x5f,0x5f,0x5f,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0xff,0xff,0xff,0xff,0xff,0x53,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x0,0x0,0x0,0x0,0x13,0x5e,0x5f,0x5f,0x8,0x25,0x0,0x5f,0x5f,0x5f,0x5f,0x5f,0x5f,0x8,0x25,0xca,0x7f,0x5f,0x5f,0x5f,0x13,0x13,0x5f,0x5f,0x5f,0x2,0xdb,0xca,0x0,0x0,0xe7,0xe6,0x66,0x65,0x0,0x0,0x0,0x0,0x44,0x3f,0xa,0xa,0xff,0xff,0xff,0xff,0xff,0x61,0x76,0x6f,0x69,0x0,0xb5,0x15,
  ?\000/___}}}}}}}}}}}}}}}}}}}}\377\377\377\377\377S\377\377\377\377\377\000\000\000\000\000\000\023^__\010%\000______\010%\312\177___\023\023___\002\333\312\000\000\347\346fe\000\000\000\000D?\012\012\377\377\377\377\377avoi\000\265\025
  artifact_prefix='./'; Test unit written to ./crash-4d3bac8a64d4e58b2f0943e6d28e6e1f16328d7d
  Base64: PwAvX19ffX19fX19fX19fX19fX19fX19fX3//////1P//////wAAAAAAABNeX18IJQBfX19fX18IJcp/X19fExNfX18C28oAAOfmZmUAAAAARD8KCv//////YXZvaQC1FQ==

ACKs for top commit:
  dergoegge:
    ACK fab164f
  brunoerg:
    ACK fab164f

Tree-SHA512: f416828f4394aa7303ee437f141e9bbd23c0e0f1b830e4ef3932338858249ba68a811b9837c5b7ad8c6ab871b6354996434183597c1a910a8d8e8d829693e4b2
@muxator muxator force-pushed the itcoin-migrate-to-v25.1-rc1 branch 2 times, most recently from 0b7b35a to fc3ac94 Compare December 5, 2023 10:38
muxator pushed a commit that referenced this pull request Jul 22, 2024
…when no change pos

d55fdb1 Move TRACEx parameters to seperate lines (Richard Myers)
2d58629 wallet: fix coin selection tracing to return -1 when no change pos (Richard Myers)

Pull request description:

  This is a bugfix for from when [optional was introduced](bitcoin@758501b)  for `change_pos` in the wallet. When optional `change_pos` is unset, we should return -1 and not 0.

  I added two new checks to the `test/functional/interface_usdt_coinselection.py` which adds coverage for the situations when `normal_create_tx_internal` and `aps_create_tx_internal` events occur with no change.

  You can reproduce this bug using the coin-selection-simulation scripts as described in [issue #16](achow101/coin-selection-simulation#16). You can also run the `interface_usdt_coinselection.py` test  without the changes to `wallet/spend.cpp`.

ACKs for top commit:
  0xB10C:
    ACK d55fdb1
  achow101:
    ACK d55fdb1
  murchandamus:
    ACK d55fdb1

Tree-SHA512: 6efac3b756bdf51debbcb759dc3c4b7a4304626bc047b70025cec02f3a04937ace7712e9558ac71e560fd136005a98c518ac5bb4b90c3282d776beccd0de9749
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.