Commit graph

41939 commits

Author SHA1 Message Date
glozow
31a3ff5515
Merge bitcoin/bitcoin#30596: fuzz: replace hardcoded numbers for bech32 limits
59c0ece0a7 fuzz: replace hardcoded numbers for bech32 limits (josibake)

Pull request description:

  Follow-up to #30047 to replace a hardcoded value that was missed in the original PR

ACKs for top commit:
  paplorinc:
    ACK 59c0ece0a7
  dergoegge:
    utACK 59c0ece0a7
  marcofleon:
    ACK 59c0ece0a7. Ran the test a bit to be sure, lgtm.
  brunoerg:
    utACK 59c0ece0a7

Tree-SHA512: 89799928feb6752a533259117340b087ff7299f9bf204b165dd87708e15b99a338521f2ac9f9e1fd91dc48b93be839059768d9e68b172e36328232174d1dfa3f
2024-08-06 15:01:08 +01:00
merge-script
43740f4971
Merge bitcoin/bitcoin#30588: depends: fix ZMQ CMake getcachesize check
a0a9a11642 depends: fix ZMQ CMake getcachesize check (fanquake)

Pull request description:

  Fixes #30587.

ACKs for top commit:
  maflcko:
    ACK a0a9a11642
  hebasto:
    ACK a0a9a11642. On Ubuntu (s390x), I was able to reproduce https://github.com/bitcoin/bitcoin/issues/30587. With this PR building `zeromq` succeeds.
  TheCharlatan:
    ACK a0a9a11642

Tree-SHA512: 70ca50ebe8e36d5a10a2354a1fbed49f5f802ae5115e09686bccca7e5d1da35168e84a6cab40dd2c83f8918889cdfdcbd5d1cbe25273b844c8ddd21865ea6c51
2024-08-06 10:30:00 +01:00
merge-script
d928f4c47f
Merge bitcoin/bitcoin#30573: Update libsecp256k1 subtree to latest master
9ec776adff Revert "build: pass --with-ecmult-gen-kb=86 to secp256k1" (fanquake)
41797f8ab9 Squashed 'src/secp256k1/' changes from 4af241b320..642c885b61 (fanquake)

Pull request description:

  Updates the libsecp256k1 subtree to 642c885b61 (which is the tag for the [`v0.5.1` release](https://github.com/bitcoin-core/secp256k1/releases/tag/v0.5.1)).
  Includes a handful of changes:
  * https://github.com/bitcoin-core/secp256k1/pull/1551
  * https://github.com/bitcoin-core/secp256k1/pull/1555
  * https://github.com/bitcoin-core/secp256k1/pull/1563
  * https://github.com/bitcoin-core/secp256k1/pull/1564
  * https://github.com/bitcoin-core/secp256k1/pull/1565
  * https://github.com/bitcoin-core/secp256k1/pull/1574

  Reverts a057869aa3 given secps default has changed (https://github.com/bitcoin-core/secp256k1/pull/1563):
  > As a rule of thumb, the default values for configuration options should target standard desktop machines and align with Bitcoin Core's defaults, and the tests should mostly exercise the default configuration (see [#1549](https://github.com/bitcoin-core/secp256k1/issues/1549#issuecomment-2200559257)).

ACKs for top commit:
  hebasto:
    ACK 9ec776adff, I've reproduced the subtree update locally with the zero diff with this PR branch.

Tree-SHA512: 903ca0ff12dcc32b6cd86aee84e19de09803d35a1ee006ce890f3761dd27f1e96fe70c7bb4c279416a96ee57c406c9627614900f0ca6f76674c0088a3d270cd2
2024-08-06 10:14:29 +01:00
josibake
59c0ece0a7
fuzz: replace hardcoded numbers for bech32 limits
Use bech32::CharLimit::BECH32 and bech32::CHECKSUM_SIZE instead of
hardcoded values. This is a follow-up fix for #34007
(where this file was missed)
2024-08-06 11:03:31 +02:00
Ava Chow
949b673472
Merge bitcoin/bitcoin#28052: blockstorage: XOR blocksdir *.dat files
fa895c7283 mingw: Document mode wbx workaround (MarcoFalke)
fa359255fe Add -blocksxor boolean option (MarcoFalke)
fa7f7ac040 Return XOR AutoFile from BlockManager::Open*File() (MarcoFalke)

Pull request description:

  Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to block requests (via P2P, RPC, REST, ...).

  Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat files when writing or reading them.

  Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat files. Any program that intentionally wants to mess with the dat files can still trivially do so.

  The XOR pattern is only applied when the blocksdir is freshly created, and there is an option to disable it (on creation), so that people can disable it, if needed.

ACKs for top commit:
  achow101:
    ACK fa895c7283
  TheCharlatan:
    Re-ACK fa895c7283
  hodlinator:
    ACK fa895c7283

Tree-SHA512: c92a6a717da83bc33a9b8671a779eeefde2c63b192362ba1d71e6535ee31d08e2802b74acc908345197de9daac6930e4771595ee25b09acd5a67f7ea34854720
2024-08-05 17:52:42 -04:00
Ava Chow
44a4a0151c
Merge bitcoin/bitcoin#30064: net: log connections failures via SOCKS5 with less severity
f3cfbd65f5 net: log connections failures via SOCKS5 with less severity (Vasil Dimov)

Pull request description:

  It is expected to have some Bitcoin nodes unreachable some of the time. A failure to connect to an IPv4 or IPv6 node is already properly logged under category=net/severity=debug. Do the same when a connection fails when using a SOCKS5 proxy. This could be either to an .onion address or to an IPv4 or IPv6 address (via a Tor exit node).

  Related: https://github.com/bitcoin/bitcoin/issues/29759

ACKs for top commit:
  achow101:
    ACK f3cfbd65f5
  mzumsande:
    Code Review ACK f3cfbd65f5
  tdb3:
    Code Review ACK f3cfbd65f5

Tree-SHA512: c6e83568783cb5233edac7840a00f708d27be9af87480fc73093ad99fe4bd8670d3f2c97fd6b6e2c54b8d9337746eacb9a5db6eefecc1486951996bfbb0a37f7
2024-08-05 17:44:12 -04:00
Ava Chow
2917f41b6a
Merge bitcoin/bitcoin#30582: doc: Add missed cmake package to build depends
ee934d093d doc: Add missed cmake package to build depends (Hennadii Stepanov)

Pull request description:

  CMake is used to build the following packages in depends when cross-compiling for Windows:
  - `libevent` (https://github.com/bitcoin/bitcoin/pull/29835)
  - `libnatpmp` (https://github.com/bitcoin/bitcoin/pull/29708)
  - `miniupnpc` (https://github.com/bitcoin/bitcoin/pull/29707)
  - `qrencode` (https://github.com/bitcoin/bitcoin/pull/29725)
  - `zeromq` (https://github.com/bitcoin/bitcoin/pull/29723)

ACKs for top commit:
  vostrnad:
    ACK ee934d093d
  achow101:
    ACK ee934d093d
  TheCharlatan:
    ACK ee934d093d
  tdb3:
    cr ut ACK ee934d093d

Tree-SHA512: 7483a680607aa218a375c285859ab19773267c81324de61f457f40057381090b15779534ff0ddb3d981341b9cd9b9e1d4afffda1ec5d5b105ad5bfcac3c7d76a
2024-08-05 17:27:37 -04:00
Ava Chow
dd7e12a3de
Merge bitcoin/bitcoin#30082: test: expand LimitOrphan and EraseForPeer coverage
172c1ad026 test: expand LimitOrphan and EraseForPeer coverage (Greg Sanders)
28dbe218fe refactor: move orphanage constants to header file (Greg Sanders)

Pull request description:

  Inspired by refactorings in #30000 as the coverage appeared a bit sparse.

  Added some minimal border value testing, timeouts, and tightened existing assertions.

ACKs for top commit:
  achow101:
    ACK 172c1ad026
  rkrux:
    reACK [172c1ad](172c1ad026)
  glozow:
    reACK 172c1ad026

Tree-SHA512: e8fa9b1de6a8617612bbe9b132c9c0c9b5a651ec94fd8c91042a34a8c91c5f9fa7ec4175b47e2b97d1320d452c23775be671a9970613533e68e81937539a7d70
2024-08-05 17:25:57 -04:00
Ava Chow
902dd14382
Merge bitcoin/bitcoin#30493: policy: enable full-rbf by default
590456e3f1 policy: enable full-rbf by default (Peter Todd)
195e98ea8e doc: add release notes for full-rbf (Peter Todd)

Pull request description:

  This pull request enables full rbf (mempool policy) by default. #28132 was closed recently with this [comment](https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2225369634).

  ---

  Rationale:

  - Full RBF config option was added in July 2022: https://github.com/bitcoin/bitcoin/pull/25353

  - It is used regularly: https://mempool.space/rbf#fullrbf

  - Most mining pools are using it: https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2059120917

ACKs for top commit:
  petertodd:
    ACK 590456e3f1
  instagibbs:
    reACK 590456e3f1
  glozow:
    reACK 590456e3f1
  achow101:
    ACK 590456e3f1
  ariard:
    tested ACK 590456e3
  murchandamus:
    reACK 590456e3f1

Tree-SHA512: 83fceef9961021687e6ff979041f89be0c616f7a49cc28a5d7edf7d8ad064fcb9c0e2af0c31f4f89867a9f6dff4e40ef8ad4dbd624e7d6a4e00ac1f1c1f66c7a
2024-08-05 16:10:46 -04:00
Ryan Ofsky
69df012e74
Merge bitcoin/bitcoin#30497: rpc: Return errors in loadtxoutset that currently go to logs
fa530ec543 rpc: Return precise loadtxoutset error messages (MarcoFalke)
faa5c86dbf refactor: Use untranslated error message in ActivateSnapshot (MarcoFalke)

Pull request description:

  The error messages should never happen in normal operation. However, if
  they do, they are helpful to return to the user to debug the issue. For
  example, to notice a truncated file.

  This fixes https://github.com/bitcoin/bitcoin/issues/28621

  Also includes a minor refactor commit.

ACKs for top commit:
  fjahr:
    Code review ACK fa530ec543
  ryanofsky:
    Code review ACK fa530ec543, just adjusting error messages a little since last review. (Thanks!)

Tree-SHA512: 224968c9b13d082ca2ed1f6a8fcc5f51ff16d6c96bd38c3679699505b54337b99cccaf7a8474391f6b11f9ccb101977b4e626898c1217eae95802e290cf105f1
2024-08-05 13:20:21 -04:00
Ryan Ofsky
21c2879f37
Merge bitcoin/bitcoin#30560: refactor: Add consteval uint256 constructor
2d9d752e4f scripted-diff: Replace uint256S("str") -> uint256{"str"} (Hodlinator)
c06f2368e2 refactor: Hand-replace some uint256S -> uint256 (Hodlinator)
b74d8d58fa refactor: Add consteval uint256(hex_str) (Hodlinator)

Pull request description:

  Motivation:
  * Validates and converts the hex string at compile time instead of at runtime into the resulting bytes.
  * Makes it possible to derive other compile time constants from `uint256`.
  * Potentially eliminates runtime dependencies (`SetHexDeprecated()` is called in less places).
  * Has stricter requirements than the deprecated `uint256S()` (requiring 64 chars exactly, disallows garbage at the end) and replaces it in a bunch of places.
  * Makes the binary smaller (tested Guix-built x86_64-linux-gnu bitcoind binary).
  * Minor: should shave off a few cycles of start-up time.

  Extracted from #30377 which diverged into exploring `consteval` `ParseHex()` solutions.

ACKs for top commit:
  maflcko:
    rebase re-cr-ACK 2d9d752e4f 🎐
  stickies-v:
    re-ACK 2d9d752e4f
  paplorinc:
    ACK 2d9d752e4f

Tree-SHA512: 39bd9320db0ed81950b5d71495eaa1d06508cc008466f2308874d70ac9ff32bc69798d2e3ef6a784868c1633fb519f60cc2111a9d0718c2663b28e78b67f7cde
2024-08-05 12:45:32 -04:00
merge-script
42326b0fa4
Merge bitcoin/bitcoin#30512: net: Log accepted connection after m_nodes.push_back; Fix intermittent test issue
fa3ea3b83c test: Fix intermittent issue in p2p_v2_misbehaving.py (MarcoFalke)
55555574d1 net: Log accepted connection after m_nodes.push_back (MarcoFalke)

Pull request description:

  Fix the two issues reported in https://github.com/bitcoin/bitcoin/pull/30468/files#r1688444784:

  * Delay a debug log line for consistency.
  * Fix an intermittent test issue.

  They are completely separate fixes, but both `net` related.

ACKs for top commit:
  0xB10C:
    Code Review ACK fa3ea3b83c
  stratospher:
    tested ACK fa3ea3b.

Tree-SHA512: cd6b6e164b317058a305a5c3e38c56c9a814a7469039e1143f1d7addfbc91b0a28506873356b373d97448b46cb6fbe94a1309df82e34c855540b241a09489e8b
2024-08-05 14:51:39 +01:00
merge-script
d15d95c5cc
Merge bitcoin/bitcoin#30575: fuzz: fix timeout in crypter target
bfd3c29e4f fuzz: fix timeout in crypter target (brunoerg)

Pull request description:

  Fixes #30503

  - Move SetKeyFromPassphrase to out of LIMITED_WHILE
  - Remove `SetKey` calls since it is already called internally by other functions.
  - Reduce number of iterations (100 is enough, no need for 10,000).

ACKs for top commit:
  maflcko:
    review ACK bfd3c29e4f 📆
  dergoegge:
    utACK bfd3c29e4f

Tree-SHA512: 275ab7d07a20bfd07279a23613678993c10c166f40cdc900213b9f4d5afb107462d5f88518a0f4ce2a52f3b7950ff2c01cf74292042f16996909fcb96f827d3e
2024-08-05 14:42:19 +01:00
fanquake
a0a9a11642
depends: fix ZMQ CMake getcachesize check
Fixes #30587.
2024-08-05 14:17:58 +01:00
Hodlinator
2d9d752e4f
scripted-diff: Replace uint256S("str") -> uint256{"str"}
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e 's/\buint256S\("(0x)?([^"]{64})"\)/uint256{"\2"}/g' $(git grep -l uint256S)
-END VERIFY SCRIPT-
2024-08-05 14:51:48 +02:00
Hodlinator
c06f2368e2
refactor: Hand-replace some uint256S -> uint256
chainparams.cpp - workaround for MSVC bug triggering C7595 - Calling consteval constructors in initializer lists fails, but works on GCC (13.2.0) & Clang (17.0.6).
2024-08-05 14:51:47 +02:00
Hodlinator
b74d8d58fa
refactor: Add consteval uint256(hex_str)
Complements uint256::FromHex() nicely in that it naturally does all error checking at compile time and so doesn't need to return an std::optional.

Will be used in the following 2 commits to replace many calls to uint256S(). uint256S() calls taking C-string literals are littered throughout the codebase and executed at runtime to perform parsing unless a given optimizer was surprisingly efficient. While this may not be a hot spot, it's better hygiene in C++20 to store the parsed data blob directly in the binary, without any parsing at runtime.
2024-08-05 14:45:18 +02:00
merge-script
1afa3c84fc
Merge bitcoin/bitcoin#30562: PayToAnchor(P2A) followups
2a3a24296e test: check that P2A with witness data is still consensus-legal (Greg Sanders)
68bd86cd7c test: P2A is a destination type with an address (Greg Sanders)

Pull request description:

  Followups for https://github.com/bitcoin/bitcoin/pull/30352

  Suggestions taken:
  https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1698542647
  https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1698563426

ACKs for top commit:
  tdb3:
    ACK 2a3a24296e
  glozow:
    ACK 2a3a24296e
  theStack:
    ACK 2a3a24296e

Tree-SHA512: 5de865b2c300fa504dbdbd5879649a6fc328da052ad8bf9479e3fea0c49c516d824908a87523ec1fb30cc536bffe2e116dd523a9b66a07f81f93429e42879f14
2024-08-05 13:27:36 +01:00
glozow
f0d08550a0
Merge bitcoin/bitcoin#30502: doc: add release notes for #22729
6d33e13bd4 doc: tor.md: use -bind=127.0.0.1:8334=onion for the Tor bind (David Gumberg)
a7f5d188cc doc: add release notes for #22729 (Vasil Dimov)

Pull request description:

  Add release notes for #22729.

ACKs for top commit:
  davidgumberg:
    reACK 6d33e13bd4
  willcl-ark:
    ACK 6d33e13bd4

Tree-SHA512: 9d7e66ee1d0bb1d75b8273707d30f20915d5040a768c2c5cd47c84997df2645c8bec35db6c09dc77ab917836622411b924373816cbc83c4be38e2e9156a139d8
2024-08-05 10:30:33 +01:00
glozow
1a19a4d960
Merge bitcoin/bitcoin#29656: chainparams: Change nChainTx type to uint64_t
bf0efb4fc7 scripted-diff: Modernize naming of nChainTx and nTxCount (Fabian Jahr)
72e5d1be1f test: Add basic check for nChainTx type (Fabian Jahr)
dc2938e979 chainparams: Change nChainTx to uint64_t (Fabian Jahr)

Pull request description:

  This picks up the work from #29331 and closes #29258.

  This simply changes the type and addresses the comments from #29331 by changing the type in all relevant places and removing unnecessary casts. This also adds an extremely simple unit test.

  Additionally this modernizes the name of `nChainTx` which helps reviewers check all use of the symbol and can make silent merge conflicts.

ACKs for top commit:
  maflcko:
    only rebase in scripted-diff, re-ACK bf0efb4fc7 🔈
  glozow:
    reACK bf0efb4fc7 via range-diff

Tree-SHA512: ee4020926d0800236fe655d0c7b127215ab36b553b04d5f91494f4b7fac6e1cfe7ee298b07c0983db5a3f4786932acaa54f5fd2ccd45f2fcdcfa13427358dc3b
2024-08-05 10:00:25 +01:00
glozow
5d682d4ba3
Merge bitcoin/bitcoin#30576: test: check that keyless P2A 'signing' via signrawtransactionwithkey succeeds
5e87f30f7c test: check that keyless P2A 'signing' via `signrawtransactionwithkey` succeeds (Sebastian Falbesoner)

Pull request description:

  This small PR adds a sanity check to verify that transactions with P2A inputs can be 'signed' successfully, using the non-wallet RPC `signrawtransactionwithkey`. Note that in the this flow, `SignStep` (which was also extended for the new `ANCHOR` output type in #30352) is never called, as signing is only tried if the locking script verification isn't successful already. See the review discussion https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1690530356 ff.

ACKs for top commit:
  instagibbs:
    ACK 5e87f30f7c
  tdb3:
    ACK 5e87f30f7c
  glozow:
    code review ACK 5e87f30f7c

Tree-SHA512: dfea75b4bf8fa0b9c265ddd63dab36374c2430c31220f0c8eb1b53dd847c183f9e1c493a0173e2da317553a1d4cb1b35aa9ffde1268c430cc610368d23b9c942
2024-08-05 09:46:13 +01:00
glozow
bba01ba18d
Merge bitcoin/bitcoin#30285: cluster mempool: merging & postprocessing of linearizations
bbcee5a0d6 clusterlin: improve rechunking in LinearizationChunking (optimization) (Pieter Wuille)
04d7a04ea4 clusterlin: add MergeLinearizations function + fuzz test + benchmark (Pieter Wuille)
4f8958d756 clusterlin: add PostLinearize + benchmarks + fuzz tests (Pieter Wuille)
0e2812d293 clusterlin: add algorithms for connectedness/connected components (Pieter Wuille)
0e52728a2d clusterlin: rename Intersect -> IntersectPrefixes (Pieter Wuille)

Pull request description:

  Part of cluster mempool: #30289

  Depends on #30126, and was split off from it. #28676 depends on this.

  This adds the algorithms for merging & postprocessing linearizations.

  The `PostLinearize(depgraph, linearization)` function performs an in-place improvement of `linearization`, using two iterations of the [Linearization post-processing](https://delvingbitcoin.org/t/linearization-post-processing-o-n-2-fancy-chunking/201/8) algorithm. The first running from back to front, the second from front to back.

  The `MergeLinearizations(depgraph, linearization1, linearization2)` function computes a new linearization for the provided cluster, given two existing linearizations for that cluster, which is at least as good as both inputs. The algorithm is described at a high level in [merging incomparable linearizations](https://delvingbitcoin.org/t/merging-incomparable-linearizations/209).

  For background and references, see [Introduction to cluster linearization](https://delvingbitcoin.org/t/introduction-to-cluster-linearization/1032).

ACKs for top commit:
  sdaftuar:
    ACK bbcee5a0d6
  glozow:
    code review ACK bbcee5a0d6
  instagibbs:
    ACK bbcee5a0d6

Tree-SHA512: d2b5a3f132d1ef22ddf9c56421ab8b397efe45b3c4c705548dda56f5b39fe4b8f57a0d2a4c65b338462d80bb5b9b84a9a39efa1b4f390420a8005ce31817774e
2024-08-05 09:42:22 +01:00
Ryan Ofsky
1a7d20509f
Merge bitcoin/bitcoin#30526: doc: Correct uint256 hex string endianness
73e3fa10b4 doc + test: Correct uint256 hex string endianness (Hodlinator)

Pull request description:

  This PR is a follow-up to #30436.

  Only changes test-code and modifies/adds comments.

  Byte order of hex string representation was wrongfully documented as little-endian, but are in fact closer to "big-endian" (endianness is a memory-order concept rather than a numeric concept). `[arith_]uint256` both store their data in arrays with little-endian byte order (`arith_uint256` has host byte order within each `uint32_t` element).

  **uint256_tests.cpp** - Avoid using variable from the left side of the condition in the right side. Credits to @maflcko: https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688273553

  **setup_common.cpp** - Skip needless ArithToUint256-conversion. Credits to @stickies-v: https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688621638

  ---

  <details>
  <summary>

  ## Logical reasoning for endianness

  </summary>

  1. Comparing an `arith_uint256` (`base_uint<256>`) to a `uint64_t` compares the beginning of the array, and verifies the remaining elements are zero.
  ```C++
  template <unsigned int BITS>
  bool base_uint<BITS>::EqualTo(uint64_t b) const
  {
      for (int i = WIDTH - 1; i >= 2; i--) {
          if (pn[i])
              return false;
      }
      if (pn[1] != (b >> 32))
          return false;
      if (pn[0] != (b & 0xfffffffful))
          return false;
      return true;
  }
  ```
  ...that is consistent with little endian ordering of the array.

  2. They have the same endianness (but `arith_*` has host-ordering of each `uint32_t` element):
  ```C++
  arith_uint256 UintToArith256(const uint256 &a)
  {
      arith_uint256 b;
      for(int x=0; x<b.WIDTH; ++x)
          b.pn[x] = ReadLE32(a.begin() + x*4);
      return b;
  }
  ```

  ### String conversions

  The reversal of order which happens when converting hex-strings <=> uint256 means strings are actually closer to big-endian, see the end of `base_blob<BITS>::SetHexDeprecated`:
  ```C++
      unsigned char* p1 = m_data.data();
      unsigned char* pend = p1 + WIDTH;
      while (digits > 0 && p1 < pend) {
          *p1 = ::HexDigit(trimmed[--digits]);
          if (digits > 0) {
              *p1 |= ((unsigned char)::HexDigit(trimmed[--digits]) << 4);
              p1++;
          }
      }
  ```
  Same reversal here:
  ```C++
  template <unsigned int BITS>
  std::string base_blob<BITS>::GetHex() const
  {
      uint8_t m_data_rev[WIDTH];
      for (int i = 0; i < WIDTH; ++i) {
          m_data_rev[i] = m_data[WIDTH - 1 - i];
      }
      return HexStr(m_data_rev);
  }
  ```
  It now makes sense to me that `SetHexDeprecated`, upon receiving a shorter hex string that requires zero-padding, would pad as if the missing hex chars where towards the end of the little-endian byte array, as they are the most significant bytes. "Big-endian" string representation is also consistent with the case where `SetHexDeprecated` receives too many hex digits and discards the leftmost ones, as a form of integer narrowing takes place.

  ### How I got it wrong in #30436

  Previously I used the less than (`<`) comparison to prove endianness, but for `uint256` it uses `memcmp` and thereby gives priority to the *lower* bytes at the beginning of the array.
  ```C++
      constexpr int Compare(const base_blob& other) const { return std::memcmp(m_data.data(), other.m_data.data(), WIDTH); }
  ```

  `arith_uint256` is different in that it begins by comparing the bytes from the end, as it is using little endian representation, where the bytes toward the end are more significant.
  ```C++
  template <unsigned int BITS>
  int base_uint<BITS>::CompareTo(const base_uint<BITS>& b) const
  {
      for (int i = WIDTH - 1; i >= 0; i--) {
          if (pn[i] < b.pn[i])
              return -1;
          if (pn[i] > b.pn[i])
              return 1;
      }
      return 0;
  }
  ```
  (The commit documents that `base_blob::Compare()` is doing lexicographic ordering unlike the `arith_*`-variant which is doing numeric ordering).

  </details>

ACKs for top commit:
  paplorinc:
    ACK 73e3fa10b4
  ryanofsky:
    Code review ACK 73e3fa10b4

Tree-SHA512: 121630c37ab01aa7f7097f10322ab37da3cbc0696a6bbdbf2bbd6db180dc5938c7ed91003aaa2df7cf4a4106f973f5118ba541b5e077cf3588aa641bbd528f4e
2024-08-04 22:27:10 -04:00
Ryan Ofsky
55d19945ef
Merge bitcoin/bitcoin#29798: Logging cleanup
a7432dd6ed logging: clarify -debug and -debugexclude descriptions (Anthony Towns)
74dd33cb0a rpc: make logging method reject "0" category and correct the help text (Vasil Dimov)
8c6f3bf163 logging, refactor: minor encapsulation improvement and use BCLog::NONE instead of 0 (Vasil Dimov)
160706aa38 logging, refactor: make category special cases explicit (Ryan Ofsky)

Pull request description:

  * Move special cases from `LOG_CATEGORIES_BY_STR` to `GetLogCategory()` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1547990373)).

  * Remove `"none"` and `"0"` from RPC `logging` help because that help text was wrong. `"none"` resulted in an error and `"0"` was ignored itself (contrary to what the help text suggested).

  * Remove unused `LOG_CATEGORIES_BY_STR[""]` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1548018694)).

  This is a followup to https://github.com/bitcoin/bitcoin/pull/29419, addressing leftover suggestions + more.

ACKs for top commit:
  LarryRuane:
    ACK a7432dd6ed
  ryanofsky:
    Code review ACK a7432dd6ed. Only changes since last review are removing dead if statement and adding AJ's suggested -debug and -debugexclude help improvements, which look accurate and much more clear.

Tree-SHA512: 41b997b06fccdb4c1d31f57d4752c83caa744cb3280276a337ef4a9b7012a04eb945071db6b8fad24c6a6cf8761f2f800fe6d8f3d8836f5b39c25e4f11c85bf0
2024-08-04 21:05:08 -04:00
Hennadii Stepanov
eb85cacd29
Merge bitcoin-core/gui#826: OptionsDialog: Allow Maximize of window
3dbd94b661 GUI/OptionsDialog: Allow Maximize of window (Luke Dashjr)

Pull request description:

ACKs for top commit:
  hebasto:
    ACK 3dbd94b661.

Tree-SHA512: 24a94840d97510ce5760c3099a765fb2f5d107d99a8f72757f509eefdaf35cb2d4d7f3243866bf6dc635fe83bb73b422e3cae2bd161d9b4b6f2e3d77bfd27353
2024-08-04 16:29:31 +01:00
Fabian Jahr
bf0efb4fc7
scripted-diff: Modernize naming of nChainTx and nTxCount
-BEGIN VERIFY SCRIPT-
sed -i 's/nChainTx/m_chain_tx_count/g' $(git grep -l 'nChainTx' ./src)
sed -i 's/nTxCount/tx_count/g' $(git grep -l 'nTxCount' ./src)
-END VERIFY SCRIPT-
2024-08-04 14:24:43 +02:00
Fabian Jahr
72e5d1be1f
test: Add basic check for nChainTx type 2024-08-04 12:12:39 +02:00
Fabian Jahr
dc2938e979
chainparams: Change nChainTx to uint64_t
Also update types of assumeutxo chainparams and some related local variables for
consistency.

Co-authored-by: russeree <reese.russell@ymail.com>
2024-08-04 12:12:38 +02:00
Anthony Towns
a7432dd6ed
logging: clarify -debug and -debugexclude descriptions 2024-08-04 06:43:01 +02:00
Vasil Dimov
74dd33cb0a
rpc: make logging method reject "0" category and correct the help text
Current logging RPC method documentation claims to accept "0" and "none"
categories, but the "none" argument is actually rejected and the "0"
argument is ignored. Update the implementation to refuse both
categories, and remove the help text claiming to support them.
2024-08-04 06:43:00 +02:00
Vasil Dimov
8c6f3bf163
logging, refactor: minor encapsulation improvement and use BCLog::NONE instead of 0
* Make the standalone function `LogCategoryToStr()` private inside
  `logging.cpp` (aka `static`) - it is only used in that file.

* Make the method `Logger::GetLogPrefix()` `private` - it is only
  used within the class.

* Use `BCLog::NONE` to initialize `m_categories` instead of `0`.
  We later check whether it is `BCLog::NONE` (in
  `Logger::DefaultShrinkDebugFile()`).
2024-08-04 06:42:59 +02:00
Ryan Ofsky
160706aa38
logging, refactor: make category special cases explicit
Make special cases explicit in GetLogCategory() and LogCategoryToStr()
functions. Simplify the LOG_CATEGORIES_BY_STR and LOG_CATEGORIES_BY_FLAG
mappings and LogCategoriesList() function.

This makes the maps `LOG_CATEGORIES_BY_STR` and `LOG_CATEGORIES_BY_FLAG`
consistent (one is exactly the opposite of the other).
2024-08-04 06:42:59 +02:00
Hodlinator
73e3fa10b4
doc + test: Correct uint256 hex string endianness
Follow-up to #30436.

uint256 string representation was wrongfully documented as little-endian due to them being reversed by GetHex() etc, and base_blob::Compare() giving most significance to the beginning of the internal array. They are closer to "big-endian", but this commit tries to be even more precise than that.

uint256_tests.cpp - Avoid using variable from the left side of the condition in the right side.

setup_common.cpp - Skip needless ArithToUint256-conversion.
2024-08-03 21:59:54 +02:00
Hennadii Stepanov
ee934d093d
doc: Add missed cmake package to build depends 2024-08-03 14:34:17 +01:00
Peter Todd
590456e3f1 policy: enable full-rbf by default
Enable full rbf (mempool policy) by default and update tests accordingly.
2024-08-02 20:22:20 +00:00
Peter Todd
195e98ea8e doc: add release notes for full-rbf 2024-08-02 20:22:20 +00:00
Sebastian Falbesoner
5e87f30f7c test: check that keyless P2A 'signing' via signrawtransactionwithkey succeeds 2024-08-02 20:13:07 +02:00
Greg Sanders
2a3a24296e test: check that P2A with witness data is still consensus-legal 2024-08-02 13:03:06 -04:00
Greg Sanders
68bd86cd7c test: P2A is a destination type with an address 2024-08-02 13:03:06 -04:00
glozow
2aff9a36c3
Merge bitcoin/bitcoin#30352: policy: Add PayToAnchor(P2A), OP_1 <0x4e73> as a standard output script for spending
75648cea5a test: add P2A ProduceSignature coverage (Greg Sanders)
7998ce6b20 Add release note for P2A output feature (Greg Sanders)
71c9b02a04 test: add P2A coverage for decodescript (Greg Sanders)
1349e9ec15 test: Add anchor mempool acceptance test (Greg Sanders)
9d89209937 policy: stop 3rd party wtxid malleability of anchor spend (Greg Sanders)
b60aaf8b23 policy: make anchor spend standard (Greg Sanders)
455fca86cf policy: Add OP_1 <0x4e73> as a standard output type (Greg Sanders)

Pull request description:

  This is a sub-feature taken out of the original proposal for ephemeral anchors #30239

  This PR makes *spending* of `OP_1 <0x4e73>` (i.e. `bc1pfeessrawgf`) standard. Creation of this output type is already standard.

  Any future witness output types are considered relay-standard to create, but not to spend. This preserves upgrade hooks, such as a completely new output type for a softfork such as BIP341.  It also gives us a bit of room to use a new output type for policy uses.

  This particular sized witness program has no other known use-cases (https://bitcoin.stackexchange.com/a/110664/17078), s it affords insufficient cryptographic security for a secure commitment to data, such as a script or a public key. This makes this type of output "keyless", or unauthenticated.

  As a witness program, the `scriptSig` of the input MUST be blank, by BIP141. This helps ensure txid-stability of the spending transaction, which may be required for smart contracting wallets. If we do not use segwit, a miner can simply insert an `OP_NOP` in the `scriptSig` without effecting the result of program execution.

  An additional relay restriction is to disallow non-empty witness data, which an adversary may use to penalize the "honest" transactor when RBF'ing the transaction due to the incremental fee requirement of RBF rules.

  The intended use-case for this output type is to "anchor" the transaction with a spending child to bring exogenous CPFP fees into the transaction package, encouraging the inclusion of the package in a block. The minimal size of creation and spending of this output makes it an attractive contrast to outputs like `p2sh(OP_TRUE)` and `p2wsh(OP_TRUE)` which
  are significantly larger in vbyte terms.

  Combined with TRUC transactions which limits the size of child transactions significantly, this is an attractive option for presigned transactions that need to be fee-bumped after the fact.

ACKs for top commit:
  sdaftuar:
    utACK 75648cea5a
  theStack:
    re-ACK 75648cea5a
  ismaelsadeeq:
    re-ACK 75648cea5a via [diff](e7ce6dc070..75648cea5a)
  glozow:
    ACK 75648cea5a
  tdb3:
    ACK 75648cea5a

Tree-SHA512: d529de23d20857e6cdb40fa611d0446b49989eaafed06c28280e8fd1897f1ed8d89a4eabbec1bbf8df3d319910066c3dbbba5a70a87ff0b2967d5205db32ad1e
2024-08-02 15:49:44 +01:00
Hennadii Stepanov
ec8b38c7b9
Merge bitcoin-core/gui#626: Showing Local Addresses in Node Window
189c987386 Showing local addresses on the Node Window (Jadi)
a5d7aff867 net: Providing an interface for mapLocalHost (Jadi)

Pull request description:

  This change adds a new row to the Node Window (debugwindow.ui)
  under the Network section which shows the LocalAddresses.

  fixes #564

  <!--
  *** Please remove the following help text before submitting: ***

  Pull requests without a rationale and clear improvement may be closed
  immediately.

  GUI-related pull requests should be opened against
  https://github.com/bitcoin-core/gui
  first. See CONTRIBUTING.md
  -->

  <!--
  Please provide clear motivation for your patch and explain how it improves
  Bitcoin Core user experience or Bitcoin Core developer experience
  significantly:

  * Any test improvements or new tests that improve coverage are always welcome.
  * All other changes should have accompanying unit tests (see `src/test/`) or
    functional tests (see `test/`). Contributors should note which tests cover
    modified code. If no tests exist for a region of modified code, new tests
    should accompany the change.
  * Bug fixes are most welcome when they come with steps to reproduce or an
    explanation of the potential issue as well as reasoning for the way the bug
    was fixed.
  * Features are welcome, but might be rejected due to design or scope issues.
    If a feature is based on a lot of dependencies, contributors should first
    consider building the system outside of Bitcoin Core, if possible.
  * Refactoring changes are only accepted if they are required for a feature or
    bug fix or otherwise improve developer experience significantly. For example,
    most "code style" refactoring changes require a thorough explanation why they
    are useful, what downsides they have and why they *significantly* improve
    developer experience or avoid serious programming bugs. Note that code style
    is often a subjective matter. Unless they are explicitly mentioned to be
    preferred in the [developer notes](/doc/developer-notes.md), stylistic code
    changes are usually rejected.
  -->

  <!--
  Bitcoin Core has a thorough review process and even the most trivial change
  needs to pass a lot of eyes and requires non-zero or even substantial time
  effort to review. There is a huge lack of active reviewers on the project, so
  patches often sit for a long time.
  -->

ACKs for top commit:
  pablomartin4btc:
    re-ACK 189c987386
  furszy:
    utACK 189c987

Tree-SHA512: 93f201bc6d21d81b27b87be050a447b841f01e3efb69b9eca2cc7af103023d7cd69eb5e16e2875855573ef51a5bf74a6ee6028636c1b6798cb4bb11567cb4996
2024-08-02 14:19:02 +01:00
merge-script
df241970a3
Merge bitcoin/bitcoin#30554: test: Avoid CScript() as default function argument
fa46a1b74b test: Avoid CScript() as default function argument (MarcoFalke)
fadf621825 test: Make leaf_script mandatory when scriptpath is set in TaprootSignatureMsg (MarcoFalke)

Pull request description:

  Unlike other function calls in default arguments, CScript should not cause any issues in the tests, because they are const.

  However, this change allows to enable the "function-call-in-default-argument (B008)" lint rule, which will help to catch severe test bugs, such as https://github.com/bitcoin/bitcoin/issues/30543#issuecomment-2259260024 .

  The lint rule will be enabled in a follow-up, when all violations are fixed.

ACKs for top commit:
  instagibbs:
    utACK fa46a1b74b
  theStack:
    lgtm ACK fa46a1b74b
  ismaelsadeeq:
    Tested ACK fa46a1b74b

Tree-SHA512: bc68b15121d50ead0fc70ad772360a7829908aedeaff8426efcb8a67f33117f67d26b4f5da94fa735dd8de9c9ff65fc10a29323f1b12f238b75486fa7cc32a89
2024-08-02 14:02:04 +01:00
brunoerg
bfd3c29e4f fuzz: fix timeout in crypter target
Move `SetKeyFromPassphrase` to out of LIMITED_WHILE,
remove `SetKey` calls since it is already called
internally by other functions and reduce the number
of iterations.
2024-08-02 09:48:10 -03:00
merge-script
9c6c667bc2
Merge bitcoin/bitcoin#30574: test: Disable known broken USDT test for now
faed533743 test: Disable known broken USDT test for now (MarcoFalke)

Pull request description:

  (cherry picked from commit faf8be7c32)

  Sadly, it still happens: https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2265205214

ACKs for top commit:
  fanquake:
    ACK faed533743

Tree-SHA512: 7108c468efd31a1f062646b7b21d69ddaaa9808cdc44db75c78d7a840830f85d016d4a95571c239402f0b6639b714224720182bcda8f53b147a0be06cfbd2b25
2024-08-02 13:26:18 +01:00
MarcoFalke
faed533743
test: Disable known broken USDT test for now
(cherry picked from commit faf8be7c32)
2024-08-02 14:02:42 +02:00
fanquake
9ec776adff
Revert "build: pass --with-ecmult-gen-kb=86 to secp256k1"
This reverts commit a057869aa3.
2024-08-02 11:33:07 +01:00
fanquake
183e2fd6b5
Update secp256k1 subtree to latest master 2024-08-02 11:32:47 +01:00
fanquake
41797f8ab9 Squashed 'src/secp256k1/' changes from 4af241b320..642c885b61
642c885b61 Merge bitcoin-core/secp256k1#1575: release: prepare for 0.5.1
cdf08c1a2b Merge bitcoin-core/secp256k1#1576: doc: mention `needs-changelog` github label in release process
40d87b8e45 release: prepare for 0.5.1
5770226176 changelog: clarify CMake option
759bd4bbc8 doc: mention `needs-changelog` github label in release process
fded437c4c Merge bitcoin-core/secp256k1#1574: Fix compilation when extrakeys module isn't enabled
763d938cf0 ci: only enable extrakeys module when schnorrsig is enabled
af551ab9db tests: do not use functions from extrakeys module
0055b86780 Merge bitcoin-core/secp256k1#1551: Add ellswift usage example
ea2d5f0f17 Merge bitcoin-core/secp256k1#1563: doc: Add convention for defaults
ca06e58b2c Merge bitcoin-core/secp256k1#1564: build, ci: Adjust the default size of the precomputed table for signing
e2af491263 ci: Switch to the new default value of the precomputed table for signing
d94a9273f8 build: Adjust the default size of the precomputed table for signing
fcc5d7381b Merge bitcoin-core/secp256k1#1565: cmake: Bump CMake minimum required version up to 3.16
9420eece24 cmake: Bump CMake minimum required version up to 3.16
16685649d2 doc: Add convention for defaults
a5269373fa Merge bitcoin-core/secp256k1#1555: Fixed O3 replacement
b8fe33332b cmake: Fixed O3 replacement
31f84595c4 Add ellswift usage example
fe4fbaa7f3 examples: fix case typos in secret clearing paragraphs (s/, Or/, or/)

git-subtree-dir: src/secp256k1
git-subtree-split: 642c885b6102725e25623738529895a95addc4f4
2024-08-02 11:32:47 +01:00
merge-script
357f195391
Merge bitcoin/bitcoin#30567: qt, build: Drop QT_STATICPLUGIN macro
7231c7630e qt: Replace deprecated LogPrintf with LogInfo in GUIUtil::LogQtInfo() (Hennadii Stepanov)
b3d3ae0680 qt, build: Drop `QT_STATICPLUGIN` macro (Hennadii Stepanov)

Pull request description:

  Broken out of https://github.com/bitcoin/bitcoin/pull/30454.

  Our `QT_STATICPLUGIN` macro is effectively equivalent to the Qt's `QT_STATIC` macro.

  It is easy to see in the `_BITCOIN_QT_IS_STATIC` macro implementation: ebd82fa9fa/build-aux/m4/bitcoin_qt.m4 (L269-L292)

  No need to handle both macros.

ACKs for top commit:
  maflcko:
    re-ACK 7231c7630e
  TheCharlatan:
    ACK 7231c7630e

Tree-SHA512: abbf21859b7ac2aaf47c5b0e075403e4cc9bc540b1565d23f51650b8932dde314586aca67fd4ed5daadebc89268baf8c18f65348fa2b836078ac24543c14cfd6
2024-08-02 11:31:29 +01:00
merge-script
8e1bd17252
Merge bitcoin/bitcoin#30544: rpc: fix maybe-uninitialized compile warning in getchaintxstats
2e86f2b201 rpc: fix maybe-uninitialized compile warning in getchaintxstats (Michael Dietz)

Pull request description:

  This resolves the compiler warning about potential uninitialized use of window_tx_count introduced in fa2dada.

  The warning:
  ```
  CXX      rpc/libbitcoin_node_a-blockchain.o
  rpc/blockchain.cpp: In function ‘getchaintxstats()::<lambda(const RPCHelpMan&, const JSONRPCRequest&)>’:
  rpc/blockchain.cpp:1742:38: warning: ‘*(std::_Optional_payload_base<unsigned int>::_Storage<unsigned int, true>*)((char*)&window_tx_count + offsetof(const std::optional<unsigned int>,std::optional<unsigned int>::<unnamed>.std::_Optional_base<unsigned int, true, true>::<unnamed>)).std::_Optional_payload_base<unsigned int>::_Storage<unsigned int, true>::_M_value’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   1742 |                 ret.pushKV("txrate", double(*window_tx_count) / nTimeDiff);
        |
  ```

ACKs for top commit:
  maflcko:
    lgtm ACK 2e86f2b201
  theStack:
    ACK 2e86f2b201
  tdb3:
    ACK 2e86f2b201

Tree-SHA512: c087e8f1cd68dd8df734a8400d30a95abe57ebd56cd53aef4230e425b33a23aa55b3af42abfd162e3be8c937a4c27e56abb70a4fedb10e2df64d52d577e0f262
2024-08-02 10:50:34 +01:00