143 Commits

Author SHA1 Message Date
MarcoFalke
bbbbaa0d9a
Fix unsigned integer overflows in interpreter 2024-10-01 10:45:44 +02:00
merge-script
5c0cd205a1
Merge bitcoin/bitcoin#29625: Several randomness improvements
ce8094246ee95232e9d84f7e37f3c0a43ef587ce random: replace construct/assign with explicit Reseed() (Pieter Wuille)
2ae392d561ecfdf81855e6df6b9ad3d8843cdfa2 random: use LogError for init failure (Pieter Wuille)
97e16f57042cab07e5e73f6bed19feec2006e4f7 tests: make fuzz tests (mostly) deterministic with fixed seed (Pieter Wuille)
2c91330dd68064e402e8eceea3df9474bb7afd48 random: cleanup order, comments, static (Pieter Wuille)
8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c net, net_processing: use existing RNG objects more (Pieter Wuille)
d5fcbe966bc501db8bf6a3809633f0b82e6ae547 random: improve precision of MakeExponentiallyDistributed (Pieter Wuille)
cfb0dfe2cf0b46f3ea9e62992ade989860f086c8 random: convert GetExponentialRand into rand_exp_duration (Pieter Wuille)
4eaa239dc3e189369d59144b524cb2808cbef8c3 random: convert GetRand{Micros,Millis} into randrange (Pieter Wuille)
82de1b80d95fc9447e64c098dcadb6b8a2f1f2ee net: use GetRandMicros for cache expiration (Pieter Wuille)
ddc184d999d7e1a87efaf6bcb222186f0dcd87ec random: get rid of GetRand by inlining (Pieter Wuille)
e2d1f84858485650ff743753ffa5c679f210a992 random: make GetRand() support entire range (incl. max) (Pieter Wuille)
810cdf6b4e12a1fdace7998d75b4daf8b67d7028 tests: overhaul deterministic test randomness (Pieter Wuille)
6cfdc5b104caf9952393f9dac2a36539d964077f random: convert XoRoShiRo128PlusPlus into full RNG (Pieter Wuille)
8cc2f45065fc1864f879248d1e1444588e27076b random: move XoRoShiRo128PlusPlus into random module (Pieter Wuille)
8f5ac0d0b608bdf396d8f2d758a792f869c2cd2a xoroshiro128plusplus: drop comment about nonexisting copy() (Pieter Wuille)
8924f5120f66269c04633167def01f82c74ea730 random: modernize XoRoShiRo128PlusPlus a bit (Pieter Wuille)
ddb7d26cfd96c1f626def4755e0e1b5aaac94d3e random: add RandomMixin::randbits with compile-known bits (Pieter Wuille)
21ce9d8658fed0d3e4552e8b02a6902cb31c572e random: Improve RandomMixin::randbits (Pieter Wuille)
9b14d3d2da05f74ffb6a2ac20b7d9efefbe29634 random: refactor: move rand* utilities to RandomMixin (Pieter Wuille)
40dd86fc3b60d7a67a9720a84a685f16e3f05b06 random: use BasicByte concept in randbytes (Pieter Wuille)
27cefc7fd6a6a159779f572f4c3a06170f955ed8 random: add a few noexcepts to FastRandomContext (Pieter Wuille)
b3b382dde202ad508baf553817c5b38fdd2d4a0c random: move rand256() and randbytes() to .h file (Pieter Wuille)
493a2e024e845e623e202e3eefe1cc2010e9b514 random: write rand256() in function of fillrand() (Pieter Wuille)

Pull request description:

  This PR contains a number of vaguely-related improvements to the random module.

  The specific changes and more detailed rationale is in the commit messages, but the highlights are:

  * `XoRoShiRo128PlusPlus` (previously a test-only RNG) moves to random.h and becomes `InsecureRandomContext`, which is even faster than `FastRandomContext` but non-cryptographic. It also gets all helper randomness functions (`randrange`, `fillrand`, ...), making it a lot more succinct to use.
  * During tests, **all** randomness is made deterministic (except for `GetStrongRandBytes`) but non-repeating (like `GetRand()` used to be when `g_mock_deterministic_tests` was used), either fixed, or from a random seed (overridden by env var).
  * Several infrequently used top-level functions (`GetRandMillis`, `GetRandMicros`, `GetExponentialRand`) are converted into member functions of `FastRandomContext` (and `InsecureRandomContext`).
  * `GetRand<T>()` (without argument) can now return the maximum value of the type (previously e.g. `GetRand<uint32_t>()` would never return 0xffffffff).

ACKs for top commit:
  achow101:
    ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce
  maflcko:
    re-ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce 🐈
  hodlinator:
    ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce
  dergoegge:
    utACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce

Tree-SHA512: 79bc0cbafaf27e95012c1ce2947a8ca6f9a3c78af5f1f16e69354b6fc9b987a28858adf4cd356dc5baf21163e9af8dcc24e70f8d7173be870e8a3ddcdd47c02c
2024-07-04 11:26:43 +01:00
MarcoFalke
fa2dada0c9
rpc: Avoid getchaintxstats invalid results 2024-07-02 08:46:02 +02:00
Pieter Wuille
6cfdc5b104 random: convert XoRoShiRo128PlusPlus into full RNG
Convert XoRoShiRo128PlusPlus into a full RandomMixin-based RNG class,
providing all utility functionality that FastRandomContext has. In doing so,
it is renamed to InsecureRandomContext, highlighting its non-cryptographic
nature.

To do this, a fillrand fallback is added to RandomMixin (where it is used by
InsecureRandomContext), but FastRandomContext still uses its own fillrand.
2024-07-01 10:26:46 -04:00
Pieter Wuille
8cc2f45065 random: move XoRoShiRo128PlusPlus into random module
This is preparation for making it more generally accessible.
2024-07-01 10:26:46 -04:00
Pieter Wuille
ddb7d26cfd random: add RandomMixin::randbits with compile-known bits
In many cases, it is known at compile time how many bits are requested from
randbits. Provide a variant of randbits that accepts this number as a template,
to make sure the compiler can make use of this knowledge. This is used immediately
in rand32() and randbool(), and a few further call sites.
2024-07-01 10:26:46 -04:00
Pieter Wuille
21ce9d8658 random: Improve RandomMixin::randbits
The previous randbits code would, when requesting more randomness than available
in its random bits buffer, discard the remaining entropy and generate new.

Benchmarks show that it's usually better to first consume the existing randomness
and only then generate new ones. This adds some complexity to randbits, but it
doesn't weigh up against the reduced need to generate more randomness.
2024-07-01 10:26:46 -04:00
Pieter Wuille
59a6df6bd5 util: add BitSet
This adds a bitset module that implements a BitSet<N> class, a variant
of std::bitset with a few additional features that cannot be implemented
in a wrapper without performance loss (specifically, finding first and
last bit set, or iterating over all set bits).
2024-06-10 07:54:48 -04:00
Ryan Ofsky
63e8fc912c ci: add getchaintxstats ubsan suppressions
Add ubsan suppressions for integer overflows in the getchaintxstats RPC.

getchainstatstx line "int nTxDiff = pindex->nChainTx - past_block.nChainTx" can
trigger ubsan integer overflows when assumeutxo snapshots are loaded, from
subtracting unsigned values and assigning the result to a signed int.

The overflow behavior probably exists in current code but is hard to trigger
because it would require calling getchainstatstx at the right time with
specific parameters as background blocks are being downloaded. But the overflow
behavior becomes easier to trigger in the upcoming commit removing fake
nChainTx values, so a suppression needs to be added before then for CI to pass.

getchainstatstx should probably be improved separately in another PR to not
need this suppression, and handle edge cases and missing nChainTx values more
carefully.
2024-03-18 11:28:40 -05:00
fanquake
217c0ce552
test: remove file-wide interpreter.cpp ubsan suppression 2024-03-02 15:26:58 -05:00
fanquake
e3b68b3b83
Merge bitcoin/bitcoin#28875: build: Pass sanitize flags to instrument libsecp256k1 code
cbea49c0d32badb975fbf22d44f8e25cc7972af7 build: Pass sanitize flags to instrument `libsecp256k1` code (Hennadii Stepanov)

Pull request description:

  This PR is a revived https://github.com/bitcoin/bitcoin/pull/27991 with an addressed [comment](https://github.com/bitcoin/bitcoin/pull/27991#discussion_r1252148488).

  Fixes https://github.com/bitcoin/bitcoin/issues/27990.

  Might be tested as follows:
  ```
  $ ./autogen.sh && ./configure --enable-fuzz --with-sanitizers=fuzzer CC=clang-13 CXX=clang++-13
  $ make clean > /dev/null && make
  $ objdump --disassemble=secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep __sanitizer_cov
   1953bd0:e8 bb c6 05 ff       call   9b0290 <__sanitizer_cov_trace_const_cmp8>
   1953d32:e8 69 c4 05 ff       call   9b01a0 <__sanitizer_cov_trace_pc_indir>
   1953d58:e8 43 c4 05 ff       call   9b01a0 <__sanitizer_cov_trace_pc_indir>
   1953d82:e8 19 c4 05 ff       call   9b01a0 <__sanitizer_cov_trace_pc_indir>
  ```

ACKs for top commit:
  fanquake:
    ACK cbea49c0d32badb975fbf22d44f8e25cc7972af7
  dergoegge:
    reACK cbea49c0d32badb975fbf22d44f8e25cc7972af7

Tree-SHA512: 801994e75b711d20eaf0d675f378da07d693f4a7de026efd93860f5f1deabed855a83eca3561725263e4fe605fcc5f91eb73c021ec91c831864e6deb575e3885
2024-01-26 11:31:34 +00:00
Hennadii Stepanov
cbea49c0d3
build: Pass sanitize flags to instrument libsecp256k1 code
Also a new UBSan suppression has been added.
2024-01-19 10:08:41 +00:00
Fabian Jahr
6044628543
crypto, hash: replace custom rotl32 with std::rotl 2024-01-05 17:12:38 +01:00
MarcoFalke
fa9dc92c53
test: Add missing CBlockPolicyEstimator::processBlockTx suppression 2023-11-29 10:51:04 +01:00
fanquake
fd30e9688e
test: migrate to some per-symbol ubsan suppressions
Tested on aarch64 using the ASAN CI job. Currently unable to test on
x86_64 due to AppArmor & podman issues.
2023-11-14 15:52:55 +00:00
fanquake
fadad10126
Merge bitcoin/bitcoin#28131: test: Add UBSan -fsanitize=integer suppressions for src/secp256k1 subtree
a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3 Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree (Hennadii Stepanov)

Pull request description:

  Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3

Tree-SHA512: 602fa3ad22d3b0f6981a51358677d2347c92c4c9f59626b497af10f7ba828ede37227d8ee717f089bf33bde5efe0854d53acc89bea46f0955e62b7f22c454d05
2023-08-01 09:40:36 +01:00
fanquake
d0c6cc4abe
suppressions: note that 'type:ClassName::MethodName' should be used
Now that the symbolizer is back in play, suppressions can once-again be
targeted to functions, rather than file-wide.
2023-07-25 13:25:55 +01:00
Hennadii Stepanov
a7477744c5
Add UBSan -fsanitize=integer suppressions for src/secp256k1 subtree 2023-07-23 14:56:51 +01:00
MarcoFalke
fa367422ef
fuzz: Bump FuzzedDataProvider.h
From fa8401f9bf/compiler-rt/include/fuzzer/FuzzedDataProvider.h
2023-07-17 09:39:52 +02:00
MarcoFalke
fae55f989e
test: Add implicit-signed-integer-truncation:*/include/c++/ suppression 2023-06-23 12:03:51 +02:00
MarcoFalke
fa76f0d0ef
refactor: Make m_count_with_* in CTxMemPoolEntry int64_t, drop UBSAN supp
This is a refactor as long as no signed integer overflow appears. In
normal operation and absent bugs, signed integer overflow should never
happen in the touched code paths.

The main benefit of this refactor is to drop the file-wide ubsan
suppression unsigned-integer-overflow:txmempool.cpp.

For now, this only changes the internal private representation and the
publicly returned type remains uint64_t.
2023-06-14 23:15:20 +02:00
Hennadii Stepanov
3ef756a5b5
Remove txmempool implicit-integer-sign-change sanitizer suppressions 2023-06-12 19:48:47 +01:00
Hennadii Stepanov
f03a708c11
doc, test: Document steps to reproduce TSan warning for libdb 2023-05-15 13:28:25 +01:00
MarcoFalke
fa15a9934e
test: Remove unused sanitizer suppressions
* The GCC suppression was fixed in gcc-11, which is available on all LTS
  releases of Linux distros.
* The feerate suppression was likely fixed and does not trigger anymore.
  If it was to trigger again, the underlying bug should be fixed instead
  of suppressing it.
* The bench suppression does not trigger anymore.

Also, add comments to tsan suppressions on how to reproduce.
2023-04-21 09:52:24 +02:00
fanquake
71b3e9b0ad
sanitizers: remove GetRNGState lsan suppression
I am no-longer seeing this. Can anyone recreate the false-positive?
2023-03-30 14:10:35 +01:00
Martin Leitner-Ankerl
5f05b27841 Add xoroshiro128++ PRNG
Xoroshiro128++ is a fast non-cryptographic random generator.
Reference implementation is available at https://prng.di.unimi.it/

Co-Authored-By: Pieter Wuille <pieter@wuille.net>
2023-01-30 18:12:21 -05:00
MarcoFalke
faa835e7e5
Revert "test: Drop no longer needed race:epoll_ctl TSan suppression"
This reverts commit a3f5e541523a843e834df1858e16f89188fe19a2.
2022-12-30 09:47:52 +01:00
Hennadii Stepanov
a3f5e54152
test: Drop no longer needed race:epoll_ctl TSan suppression 2022-12-27 18:33:34 +00:00
MacroFake
fadb714039
test: Remove unused sanitizer suppressions 2022-11-21 12:13:42 +01:00
Hennadii Stepanov
c8dc0e3eaa
refactor: Inline CTxMemPoolEntry class's functions 2022-11-16 20:17:05 +00:00
Hennadii Stepanov
75bbe594e5
refactor: Move CTxMemPoolEntry class to its own module
This change nukes the policy/fees->mempool circular dependency.

Easy to review using `diff --color-moved=dimmed-zebra`.
2022-11-16 20:16:07 +00:00
James O'Beirne
00eeb31c76 scripted-diff: rename CChainState -> Chainstate
-BEGIN VERIFY SCRIPT-
sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*')
-END VERIFY SCRIPT-

Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-09-09 11:47:27 -04:00
MarcoFalke
fa07f84e31
Fix signed integer overflow in prioritisetransaction RPC 2022-06-22 09:32:09 +02:00
MarcoFalke
fa097d074b
addrman: Log too low compat value
Also remove uint8_t{} casts from values that are already of the same
type.
2022-02-25 14:16:32 +01:00
MarcoFalke
aaaa4dbab4
Avoid implicit-integer-sign-change in bech32.cpp 2022-02-25 09:43:54 +01:00
MarcoFalke
fae6b26758
test: Remove no longer needed suppressions 2022-02-25 09:41:05 +01:00
MarcoFalke
fa7e1471c0
test: Fix intermittent Tsan issue 2022-02-23 09:34:20 +01:00
Hennadii Stepanov
81738d2881
test: Remove suppression no longer needed with headers-only Boost.Test 2022-02-19 12:40:00 +02:00
fanquake
e0367e84b3
Merge bitcoin/bitcoin#24301: build: header-only Boost
5d399f9f3df513a0400049238f5ef0ef2352d57e build: remove native B2 package (fanquake)
2037a3b6c1222d2802ff7c8463f2bb79ba8b57d8 build: header-only Boost (fanquake)
39e66e938fb688f5400ad94a1b317fcc2a87bc31 build: use header-only Boost unit test (fanquake)

Pull request description:

  This PR converts our Boost usage to header only. We switch from using our last remaining Boost lib (unit test), to using it's header-only implementation (see https://www.boost.org/doc/libs/1_78_0/libs/test/doc/html/boost_test/adv_scenarios/single_header_customizations/multiple_translation_units.html).

  Also related to #24291.

  Guix build:
  ```bash
  ```

ACKs for top commit:
  hebasto:
    re-ACK 5d399f9f3df513a0400049238f5ef0ef2352d57e
  MarcoFalke:
    approach ACK 5d399f9f3df513a0400049238f5ef0ef2352d57e 📞

Tree-SHA512: e60835ee9c11aa941a64679616da2002d6cd86e464895372fafdd42ad6499d7eb1dde6f0013c60adaeb97bd191198430cb158a7a7417b38080dd7106b28e3ba5
2022-02-14 10:04:17 +00:00
fanquake
39e66e938f
build: use header-only Boost unit test 2022-02-13 20:59:02 +00:00
MarcoFalke
fa6065661a
refactor: Avoid unsigned integer overflow in core_write 2022-02-11 17:21:44 +01:00
MarcoFalke
fa2807ef19
test: Remove unused integer sanitizer suppressions 2022-02-09 21:26:25 +01:00
MarcoFalke
fac62056b5
Fix integer sanitizer suppressions in validation.cpp 2022-02-07 15:20:36 +01:00
MarcoFalke
fad81548fa
test: Avoid testing negative block heights 2022-02-02 15:32:06 +01:00
MarcoFalke
219d728fcb
Merge bitcoin/bitcoin#24219: Fix implicit-integer-sign-change in bloom
fad84a25956ec081f22aebbda309d168a3dc0004 refactor: Fixup uint64_t-cast style in touched line (MarcoFalke)
fa041878de786f5be74ec74a06ec407c99ca8656 Fix implicit-integer-sign-change in bloom (MarcoFalke)

Pull request description:

  Signed values don't really make sense when using `std::vector::operator[]`.

  Fix that and remove the suppression.

ACKs for top commit:
  PastaPastaPasta:
    utACK fad84a25956ec081f22aebbda309d168a3dc0004
  theStack:
    Code-review ACK fad84a25956ec081f22aebbda309d168a3dc0004

Tree-SHA512: 7139dd9aa098c41e4af1b6e63dd80e71a92b0a98062d1676b01fe550ffa8e21a5f84a578afa7a536d70dad1b8a5017625e3a9e2dda6f864b452ec77b130ddf2a
2022-02-02 15:00:22 +01:00
MarcoFalke
36f8e99d24
Merge bitcoin/bitcoin#24218: zmq: Fix implicit-integer-sign-change
fa2406a50a83184d101d1bb3f2b282ae280370ba zmq: Fix implicit-integer-sign-change (MarcoFalke)

Pull request description:

  uint256::begin() returns unsigned data, so there is no reason to make it signed.

  Fix that and remove the sanitizer suppression.

ACKs for top commit:
  hebasto:
    ACK fa2406a50a83184d101d1bb3f2b282ae280370ba
  PastaPastaPasta:
    utACK fa2406a50a83184d101d1bb3f2b282ae280370ba, I have reviewed the code and think it makes sense

Tree-SHA512: 150ebcf3fdc3e0f60b6fd8e5fe638737b01e8a0863296bd545fb5ed17d33ab23b2ff94204996aa7b4617650b7383bd86ed2d2bf46746b410feae449de179a2bd
2022-02-01 10:18:53 +01:00
MarcoFalke
fcac16fff8
Merge bitcoin/bitcoin#24190: test: Fix sanitizer suppresions in streams_tests
faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975 test: Fix sanitizer suppresions in streams_tests (MarcoFalke)

Pull request description:

  Two changes (that also make sense on their own) to remove the file-wide sanitizer suppression:

  * `FindByte` no longer takes a `char`, but an `uint8_t`, after commit 196b4599201dbce3e0317e9b98753fa6a244b82d.
  * The `key` vector of unsigned chars can be removed and inlined as initializer-list. This avoids a bunch of verbose code like `clear()` and `push_back` of `char`s.

ACKs for top commit:
  PastaPastaPasta:
    utACK faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975, I have reviewed the changes and agree it makes sense to merge

Tree-SHA512: 747b9d4676fad6d07f3955668639c93333625e69199ff4c499f01167de3875990d93db85e775a7f5b1b684575dceaec8aa000b4db15525fc47b699bac1c85e3d
2022-02-01 09:42:34 +01:00
MarcoFalke
fa041878de
Fix implicit-integer-sign-change in bloom 2022-01-31 17:23:54 +01:00
MarcoFalke
fa2406a50a
zmq: Fix implicit-integer-sign-change 2022-01-31 16:53:12 +01:00
MarcoFalke
1111d33532
refactor: Make MessageBoxFlags enum underlying type unsigned 2022-01-31 09:27:12 +01:00