[go: up one dir, main page]

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

uvhw:patch-4 #124

Open
wants to merge 10,000 commits into
base: lows
Choose a base branch
from
Open

uvhw:patch-4 #124

wants to merge 10,000 commits into from

Conversation

uvhw
Copy link
@uvhw uvhw commented Feb 9, 2022

hebasto and others added 30 commits January 12, 2022 17:38
b5c9bb5 util: Restore GetIntArg saturating behavior (James O'Beirne)

Pull request description:

  The new locale-independent atoi64 method introduced in bitcoin#20452 parses large integer values higher than maximum representable value as 0 instead of the maximum value, which breaks backwards compatibility. This commit restores compatibility and adds test coverage for this case in terms of the related GetIntArg and strtoll functions.

  Specifically, command line or bitcoin.conf integer values greater than `9223372036854775807` (`2**63-1`) used to be parsed as `9223372036854775807` before bitcoin#20452. Then bitcoin#20452 caused them to be parsed as `0`. And after this PR they will be parsed as `9223372036854775807` again.

  This change is a stripped-down alternative version of bitcoin#23841 by jamesob

ACKs for top commit:
  jamesob:
    Github ACK bitcoin@b5c9bb5
  vincenzopalazzo:
    ACK bitcoin@b5c9bb5
  MarcoFalke:
    review ACK b5c9bb5 🌘

Tree-SHA512: 4e8abdbabf3cf4713cf5a7c5169539159f359ab4109a4e7e644cc2e9b2b0c3c532fad9f6b772daf015e1c5340ce59280cd9a41f2730afda6099cbf636b7d23ae
…not installed

18f304d build: Improve error message when pkg-config is not installed (Hennadii Stepanov)

Pull request description:

  Fixes bitcoin#24037.

  With this PR:
  ```
  # ./autogen.sh
  configure.ac:16: error: PKG_PROG_PKG_CONFIG macro not found. Please install pkg-config and re-run autogen.sh
  configure.ac:16: the top level
  autom4te: /usr/bin/m4 failed with exit status: 1
  aclocal: error: /usr/bin/autom4te failed with exit status: 1
  autoreconf: aclocal failed with exit status: 1
  ```

ACKs for top commit:
  laanwj:
    Tested ACK 18f304d
  jarolrod:
    ACK 18f304d

Tree-SHA512: ba845f44c966fea6cf7cee0db9cacc431072e2005ad065c8f2bbe3cffd8415c3af6ed443cccf9606df7de4df2ff3e72636afb5f3776d2a96af8572aab7018549
PoissonNextSend is used by net and net_processing and is stateless, so
place it in the utility random.cpp translation unit.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
This distribution is used for more than just the next inv send, so make
the name more generic.

Also rename to "exponential" to avoid the confusion that this is a
poisson distribution.

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

ren  PoissonNextSend   GetExponentialRand
ren  "a poisson timer" "an exponential timer"
-END VERIFY SCRIPT-
This test can now be run even with the Bitcoin Core wallet disabled.
…eer for new libevent

c62d763 Necessary improvements to make configure work without libevent installed (Perlover)
091ccc3 The evhttp_connection_get_peer function from libevent changes the type of the second parameter. Fixing the problem. (Perlover)

Pull request description:

  The second parameter of evhttp_connection_get_peer in libevent already has type as `const char **`
  The compilation of bitcoind with the fresh libevent occurs errors

  Details: bitcoin#23606

ACKs for top commit:
  laanwj:
    Code review ACK c62d763
  luke-jr:
    tACK c62d763

Tree-SHA512: d1c8062d90bd0d55c582dae2c3a7e5ee1b6c7ca872bf4aa7fe6f45a52ac4a8f59464215759d961f8efde0efbeeade31b08daf9387d7d50d7622baa1c06992d83
aa8a65e test: use MiniWallet for mempool_accept.py (Sebastian Falbesoner)
b24f6c6 test: MiniWallet: support default `from_node` for creating txs (Sebastian Falbesoner)
f30041c test: create txs with current `nVersion` (2) by default (Sebastian Falbesoner)
2f79786 test: refactor: add constant for sequence number `SEQUENCE_FINAL` (Sebastian Falbesoner)

Pull request description:

  This PR enables one more of the non-wallet functional tests (mempool_accept.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in bitcoin#20078.

  It also includes some other minor changes that came up while working on the replacement:
  * [commit 1/4] replace magic number 0xffffffff for a tx's nSequence with a new constant `SEQUENCE_FINAL`
  * [commit 2/4] create `CTransaction` instances with the current nVersion=2 by default, in order to use BIP68 for tests
  * [commit 3/4] support default `from_node` parameter for creating txs (this is a stripped down version of PR bitcoin#24025)

ACKs for top commit:
  MarcoFalke:
    re-ACK aa8a65e 📊

Tree-SHA512: 34cd085ea4147ad5bd3f3321c84204064ceb95f382664c7fe29062c1bbc79d9d9465c5e46d35e11c416f2f3cd46030c90a09b518c829c73ae40d060be5e4c9cb
Can be reviewed with
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
-BEGIN VERIFY SCRIPT-
sed -i 's/m_cs_callbacks_pending/m_callbacks_mutex/g' ./src/scheduler.h ./src/scheduler.cpp
-END VERIFY SCRIPT
In each of the critical sections, only the the guarded variables are
accessed, without any chance that within one section another one is
called.  Hence, we can use an ordinary Mutex instead of RecursiveMutex.
On a period boundary, getdeploymentinfo (and previously getblockchaininfo)
would report the status and statistics for the next block rather than
the current block. Change this to always report the status/statistics
of the current block, but add status-next to report the status for the
next block.
-BEGIN VERIFY SCRIPT-
sed -i 's/std::string m_command;/std::string m_type;/g' ./src/net.h
sed -i 's/* command and size./* type and size./g' ./src/net.h
sed -i 's/msg.m_command/msg.m_type/g' ./src/net.cpp ./src/net_processing.cpp ./src/test/fuzz/p2p_transport_serialization.cpp
-END VERIFY SCRIPT-
-BEGIN VERIFY SCRIPT-
sed -i 's/cs_SubVer/m_subver_mutex/g' ./src/net.h ./src/net.cpp ./src/net_processing.cpp
-END VERIFY SCRIPT-
In each of the critical sections, only the the guarded variable is
accessed, without any chance that within one section another one is
called.  Hence, we can use an ordinary Mutex instead of RecursiveMutex.
fanquake and others added 24 commits February 5, 2022 10:58
…std::string

824e1ff bench: Represents paths with fs::path instead of std::string (Ryan Ofsky)

Pull request description:

  Suggested bitcoin#20744 (comment)

ACKs for top commit:
  fanquake:
    untested ACK 824e1ff
  hebasto:
    ACK 824e1ff, tested on Linux Mint 20.2 (x86_64).

Tree-SHA512: 348fc189f30b5ad9a8e49e95e535d2c044462a9d534c3f34d887fbde0c05c41e88e02b4fc340709e6395a1188496a8333eb9e734310aa4c41755ec080e53c06e
bitcoin_config.h is auto-generated by the msvc-autogen.py script.
d570f26 Add bitcoin_config.h to build_msvc/.gitignore (Hennadii Stepanov)

Pull request description:

  `bitcoin_config.h` is auto-generated by the `msvc-autogen.py` script.

ACKs for top commit:
  sipsorcery:
    ACK d570f26
  vincenzopalazzo:
    ACK bitcoin@d570f26

Tree-SHA512: 913a04733454e3f9309db19e53a9f499fb304a9d1bff5aa51f2b2938adbd67d35ddacb31c5a2cec8790a060b1ee48c546bc3e755ee4945023b23fc9236deb3e4
…#20744

d216bc8 Re-enable walletinit_verify_walletdir_no_trailing2 test disabled in bitcoin#20744 (Ryan Ofsky)
80cd64e Re-enable util_datadir check disabled in bitcoin#20744 (Ryan Ofsky)

Pull request description:

  Reenable some broken tests as discussed bitcoin#20744 (comment) and bitcoin#20744 (comment)

  Fix windows test cases broken in bitcoin#20744, by passing normalized path arguments to fs::equivalent, fs::exists, and fs::is_directory, instead of non-normalized arguments. Also re-enable the tests.

  It is possible these changes also fix real init behavior on windows when -datadir or -walletdir paths with trailing dots or dashes are used, but it's not clear because I only tested on wine.

ACKs for top commit:
  hebasto:
    ACK d216bc8, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 2099ddfa1a3ad70f7ac2ff413929414a1851d257b280da25c0f5cefb46fd1372b580a1f1ee5280681a1c16e6031f119185cadd4f7a6121298562cf001f711068
Compiled with some libstdc++ versions (e.g., on Ubuntu 20.04)
std::filesystem:::create_directories() call fails to handle symbol links
properly.
Before 7cedafc added the TREE
section, this line appeared right after the KEY section.

It doesn't really fit in its former location since it's the KEY
section that discusses derivation path syntax, not the TREE section.
…ation weight calculation

fadc54b Fix unsigned integer overflow in tapscript validation weight calculation (MarcoFalke)

Pull request description:

  Change the tapscript validation weight constants from uint64_t to int64_t, since the type of m_validation_weight_left is also int64_t. Otherwise this will cause sanitizer warnings.

  This should be safe because signed integer overflow isn't expected to happen.

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

Tree-SHA512: 7a62d3a84733ab7827e3fa50d83f5493f2481b725c587e986eb2c128a769f023756f3ad964401526e386a847aa630a9f6c43a57d25ce5fd4af0b6bb5e0615528
fad8154 test: Avoid testing negative block heights (MarcoFalke)

Pull request description:

  A negative chain height is only used to denote an empty chain, not the height of any block.

  So stop testing that and remove a suppression.

ACKs for top commit:
  brunoerg:
    crACK fad8154

Tree-SHA512: 0f9e91617dfb6ceda99831e6cf4b4bf0d951054957c159b1a05a178ab6090798fae7368edefe12800da24585bcdf7299ec3534f4d3bbf5ce6a6eca74dd3bb766
fadcd03 Fix unsigned integer overflow in LoadMempool (MarcoFalke)

Pull request description:

  It doesn't seem ideal to have an integer sanitizer enabled, but then disable it for the whole validation.cpp file.

  This removes one of the two violations.

  This should be a refactor.

ACKs for top commit:
  prayank23:
    Code Review ACK bitcoin@fadcd03

Tree-SHA512: 9fb2f3d49008a59cd45b7c17be0c88c04e61183197c11c8176865af5532c8d0c940db49a351dd0fc75e1d7fd8678c3b816d34cfca170dc6b9cf8f37fdf1c8cae
It returns an incorrect result when called with a Decimal,
for which the "//" operator works differently.
Also drop unnecessary call to satoshi_round.
…nner

a036358 test: Repair failfast option for test runner (Martin Zumsande)

Pull request description:

  Fixes bitcoin#23990

  After bitcoin#23799, the `--failfast` option in the test runner for the functional tests stopped working, because a second outer loop was introduced, which would have needed a `break` too for the test runner to fail immediately. This also led to the errors reported in bitcoin#23990.

  This provides a straightforward fix for that.
  There is also bitcoin#23995 which is a larger refactor, but that hasn't been updated in a while to fix the failfast issue.

ACKs for top commit:
  pg156:
    Tested ACK a036358. I agree adding the `all_passed` flag to break out of the outer loop when needed makes sense. The "failfast" option works after this change.

Tree-SHA512: 3e2f775e36c13d180d32a05cd1cfe0883274e8615cdbbd4e069a9899e9b9ea1091066cf085e93f1c5326bd8ecc6ff524e0dad7c638f60dfdb169fefcdb26ee52
d1fab9d test: Call ceildiv helper with integer (Martin Zumsande)

Pull request description:

  On master,

  `assert_fee_amount(Decimal("0.00000993"), 217, Decimal("0.00004531"))` passes
  `assert_fee_amount(Decimal("0.00000993"), Decimal("217"), Decimal("0.00004531"))` fails.

  the reason is that the // operator in  `ceildiv(a,b) = -(-a//b)`  has a different behavior for Decimals, see [doc](https://docs.python.org/3/library/decimal.html#decimal-objects).

  `wallet_send.py` calls this function with Decimals, and I think this is the reason for the failure reported in the OP of bitcoin#24151 (`wallet_send.py --legacy-wallet` line 332, the numbers used in the example above are from there). However, the other failures reported there cannot be explained by this, so this is just a partial fix.

ACKs for top commit:
  ryanofsky:
    Code review ACK d1fab9d. Tracking down this problem was a good find, and code seems safer and easier to understand now

Tree-SHA512: 5bf0568cd1a0824f6b1a15a03580b6e9391b4f51112a97c1d00469d255bf6dda45c49a36fa567a5ba9b9973efe1d9cdd480db91965c9f4c2aa963629a8a32cba
No need to specify `$(package)_download_file` when it is equal to
`$(package)_file_name`.
fa4b619 test: Remove unused valgrind suppressions (MarcoFalke)
faccb2d test: Exclude broken feature_init for now (MarcoFalke)
fa086d8 test: Properly skip feature_syscall_sandbox in valgrind (MarcoFalke)

Pull request description:

ACKs for top commit:
  fanquake:
    ACK fa4b619

Tree-SHA512: 5be1a8f288182d386531a033ae7258f753dd655dfa1746a52b65622a0359c2b7143a25b49c0747538308eed606a691847d2f59a5a0382b7751b8de7172adf0d3
…oser to KEY section

bac30e8 docs: Move explanation of hardened key syntax closer to KEY section (Bitcoin Hodler)

Pull request description:

  The line about "(Anywhere a `'` suffix is permitted to denote hardened derivation, the suffix `h` can be used instead.)" belongs with the section on KEY expressions, not following the unrelated TREE section.

ACKs for top commit:
  prusnak:
    ACK bac30e8
  meshcollider:
    ACK bac30e8

Tree-SHA512: 56fe97b89c02e67e94cab33b01e56f17f9b501b97036c5b35939dc4000a9d5e9afe4333869ba97bbe81372c538b7b2021a7d2520aba731400d8d0e62714d52b4
…tories() call

b9c113a util: Avoid buggy std::filesystem:::create_directories() call (Hennadii Stepanov)

Pull request description:

  Compiled with some libstdc++ versions (e.g., on Ubuntu 20.04) [`std::filesystem:::create_directories()`](https://en.cppreference.com/w/cpp/filesystem/create_directory) call [fails](bitcoin#24257 (comment)) to handle symbol links properly.

  No behavior change in comparison to the [pre-20744](bitcoin@c194293) master branch.

  Fixes bitcoin#24257.

ACKs for top commit:
  ryanofsky:
    Code review ACK b9c113a. Nice simplification and fix
  MarcoFalke:
    review ACK b9c113a 🐬

Tree-SHA512: 79d940cfc1f68d9b0548fb2ab005e90850b54ac0fb3bb2940afd632d56288d92687579a3176bac3fd0ea3d2dae71e26444f8f7bdb87862414c12866ae5e857c4
99de806 validation: use stronger EXCLUSIVE_LOCKS_REQUIRED() (Vasil Dimov)

Pull request description:

  bitcoin#24103 added annotations to
  denote that the callers of `CChainState::ActivateBestChain()` and
  `CChainState::InvalidateBlock()` must not own `m_chainstate_mutex` at
  the time of the call.

  Replace the added `LOCKS_EXCLUDED()` with a stronger
  `EXCLUSIVE_LOCKS_REQUIRED()`, see
  https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative for the
  difference between both.

ACKs for top commit:
  hebasto:
    ACK 99de806.
  jonatack:
    ACK 99de806. Tested with Debian clang version 13.0.1.  Reproduced hebasto's results. Verified that  `LoadExternalBlockFile()` needs the annotation added here.

Tree-SHA512: 59640d9ad472cdb5066ecde89cc0aff8632a351fc030f39bb43800d2c856fb1aed3576e4134212d32be161b18780f06dc5066ac71df7f7cd69e3f21f886e1542
…ineEdit

aeb18b6 Bugfix: GUI: Check validity when QValidatedLineEdit::setText is called (Luke Dashjr)
b1a544b Bugfix: GUI: Re-check validity after QValidatedLineEdit::setCheckValidator (Luke Dashjr)
2385b50 Bugfix: GUI: Only apply invalid style to QValidatedLineEdit, not its tooltip (Luke Dashjr)

Pull request description:

  1. Use a CSS selector to avoid changing the background colour of the tooltip.
  2. Re-check validity of input when we first set the validator (probably a no-op in practice).
  3. Check validity of input when it is set programmatically via `setText` (eg, via the address book). Probably no-op in practice UNTIL merging bitcoin#15987 or any other PR that adds a warning for valid addresses.

  Moved from bitcoin#18133 (just concept ACKs)

ACKs for top commit:
  Sjors:
    tACK aeb18b6
  hebasto:
    ACK aeb18b6, tested on Linux Mint 20.3 (Qt 5.12.8).

Tree-SHA512: b6fa8ee4dec76e1c759095721240e6fa5071a02993cb28406e96a0fa2e819f5dddc03d2e7c9073354d7865c2b09eb263afaf853ecba42e9fc4f50ef4ae20bf0f
….cpp

fac6205 Fix integer sanitizer suppressions in validation.cpp (MarcoFalke)

Pull request description:

  It doesn't seem ideal to have an integer sanitizer enabled, but then disable it for the whole validation.cpp file.

  Fix it with a refactor and remove the suppression.

ACKs for top commit:
  hebasto:
    ACK fac6205, I have reviewed the code and it looks OK, I agree it can be merged.
  prayank23:
    Code Review ACK bitcoin@fac6205

Tree-SHA512: efc5b9887cb2e207033b264ebf425bae5ff013e909701c049aea5d79a21f10495826e962d171b3d412717cbf0a4723e5124133b5401b35a73915212e85e91020
…load_file` assignments

d644c45 build, refactor: Drop redundant `$(package)_download_file` assignments (Hennadii Stepanov)

Pull request description:

  No need to specify `$(package)_download_file` when it is equal to `$(package)_file_name`.

  Historically, before bitcoin#19817, distinct `$(package)_download_file` and `$(package)_file_name` were used for better portability (I guess) by removing `+` characters from a file name.

  The only package which still use file renaming is `native_capnp`: https://github.com/bitcoin/bitcoin/blob/eca694a4e78d54ce4e29b388b3e81b06e55c2293/depends/packages/native_capnp.mk#L3-L5

ACKs for top commit:
  shaavan:
    ACK d644c45
  fanquake:
    ACK d644c45

Tree-SHA512: 488dd0f55cea077174e78a75d8385bacb1a5463883cadeb5fd7c9426865ea5f3a8bad0bd6e8e9d530bce6f0c1715349b3fbabb4e22634348cdd68f5fc8a3c53b
fa1b227 Remove broken and unused CDataStream methods (MarcoFalke)
faee5f8 test: Create fresh CDataStream each time (MarcoFalke)
fa71114 test: Inline expected_xor (MarcoFalke)

Pull request description:

  The `insert` and `erase` methods have many issues:

  * They are unused
  * They are confusing and hard to read, as they implement "special cases" for optimization, that isn't needed
  * They are broken (See bitcoin#24231)
  * Fixing them leads to mingw compile errors (See bitcoin#24231 (comment))

  Fix all issues by removing them

ACKs for top commit:
  laanwj:
    Code review ACK fa1b227

Tree-SHA512: 9d9e5d42e6ffc5ae82bdb67cfb5b50b45977ae674acee6ff99092560aebf2fc7e4584ded614e190db0663226fa198e34350517cd7ee57d518de22e9568bc349a
Copy link
Author
@uvhw uvhw left a comment

Choose a reason for hiding this comment

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

Batching blocks 5, 6, and 7

curl 'https://api.blockcypher.com/v1/btc/main/blocks/5;6;7'

[{
"hash": "000000003031a0e73735690c5a1ff2a4be82553b2a12b776fbd3a215dc8f778d",
"height": 6,
"chain": "BTC.main",
"total": 0,
"fees": 0,
"ver": 1,
"time": "2009-01-09T03:29:49Z",
...,
},
{
"hash": "000000009b7262315dbf071787ad3656097b892abffd1f95a1a022f896f533fc",
"height": 5,
"chain": "BTC.main",
"total": 0,
"fees": 0,
"ver": 1,
"time": "2009-01-09T03:23:48Z",
...,
},
{
"hash": "0000000071966c2b1d065fd446b1e485b2c9d9594acd2007ccbd5441cfc89444",
"height": 7,
"chain": "BTC.main",
"total": 0,
"fees": 0,
"ver": 1,
"time": "2009-01-09T03:39:29Z",
...,
}]

Copy link
Author
@uvhw uvhw left a comment

Choose a reason for hiding this comment

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

@uvhw uvhw changed the base branch from lows to 201709_multihash February 22, 2022 06:21
@uvhw uvhw changed the base branch from 201709_multihash to lows February 22, 2022 06:21
@uvhw uvhw changed the title Rename pkg.m4 to uvhw/pkg.m4 uvhw:patch-4 Feb 22, 2022
Copy link
Author
@uvhw uvhw left a comment

Choose a reason for hiding this comment

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

its ok
#124
" I sign important messages with the address 1GwvLW9qJ8uaYjew3cFvPiqxViWhuU1pKT"

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.