04fee75bac Use ReadLE64 in uint256::GetUint64() instead of duplicating logic (Pieter Wuille)
Pull request description:
No need to have a (naive) copy of the `ReadLE64` logic inside `uint256::GetUint64`, when we have an optimized function for exactly that.
ACKs for top commit:
davidgumberg:
ACK 04fee75bac
jonatack:
ACK 04fee75bac review, this use of ReadLE64() is similar to the existing invocation by Num3072::Num3072(), sanity checked that before and after this change GetUint64() returns the same result (debug build, clang 13)
Tree-SHA512: 0fc2681536a18d82408411bcc6d5c6445fb96793fa43ff4021cd2933d46514c725318da35884f428d1799023921f33f8af091ef428ceb96a50866ac53a345356
8b2891a6d1 i2p: use the same destination type for transient and persistent addresses (Vasil Dimov)
Pull request description:
We generate our persistent I2P address with type `EdDSA_SHA512_Ed25519` (`DEST GENERATE SIGNATURE_TYPE=7`).
Use the same type for our transient addresses which are created by the `SESSION CREATE ...` command. If not specified, then the default one is `DSA_SHA1` according to https://geti2p.net/en/docs/api/samv3.
Fixes https://github.com/bitcoin/bitcoin/issues/26062
ACKs for top commit:
mzumsande:
ACK 8b2891a6d1
sipa:
utACK 8b2891a6d10f4a3875010d2e8eafd78bcf378952; didn't test but verified this matches the documentation
Tree-SHA512: 1b10e7e1e274b77609d08ee9cf9d73fef8c975c51aec452ce23e15fcf41709398c697087bfdece121b1fd26bc0501fc45857a91aaab679cadd0cbb37dd94c3a7
b0349a7d95 doc: consolidate & remove release-note fragments (fanquake)
Pull request description:
The formatting / ordering doesn't matter a great deal here, given this will shortly be moving to the wiki for further additions / formatting changes etc.
ACKs for top commit:
MarcoFalke:
ACK b0349a7d95
achow101:
ACK b0349a7d95
jarolrod:
ACK b0349a7d95
Tree-SHA512: 80d2f9f25bc13d407ab9fd0473ff02043a1e6b9895e27d3229d717d357606063472582d31bc1b4058741d2e34be806ef1460acd66d43d6493562ffcf3f6defa5
02c9e56468 fs: fully initialize _OVERLAPPED for win32 (Cory Fields)
Pull request description:
```bash
fs.cpp: In member function ‘bool fsbridge::FileLock::TryLock()’:
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Werror=missing-field-initializers]
129 | _OVERLAPPED overlapped = {0};
| ^
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Werror=missing-field-initializers]
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Werror=missing-field-initializers]
```
Came up in #25972. That PR is now rebased on this change.
Closes: #26006
ACKs for top commit:
sipsorcery:
tACK 02c9e56468.
hebasto:
ACK 02c9e56468, tested on Linux x86_64:
Tree-SHA512: 6a0495c34bd952b2bb8c994a1450da7d3eee61225bb4ff0ce009c013f5e29dba94bb1c3ecef9989dc18c939909fdc8eba690a38f96da431ae9d64c23656de7d0
4b656b9ed3 build: remove unused libevent cppflags (fanquake)
afce044bb6 build: remove unused natpmp / upnp cppflags (fanquake)
Pull request description:
Their inclusion is likely just the result of copy-paste.
The only place upnp & natpmp CPPFLAGS should be used is [`libbitcoin_node` (mapport.cpp)](13fd9ee5c2/src/Makefile.am (L352)).
ACKs for top commit:
theuni:
ACK 4b656b9ed3
hebasto:
ACK 4b656b9ed3, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 800daeb06ddcbade3a862ca939a8cf87dc36282ed4fe9dc256d17b78a8751e2e08799788dc449046e1d875f93372912269d3ffcb5702628a41648794df32e887
By throwing a custom exception from `Univalue::checkType` (instead of a plain
std::runtime_error) and catching it on the RPC server request handler.
So we properly return RPC_TYPE_ERROR (-3) on arg type errors and
not the general RPC_MISC_ERROR (-1).
and also fix spelling in test/lint/lint-locale-dependence.py not caught by the
spelling linter and fix up a paragraph we are touching here in test/README.md.
The `sendall` RPC doesn't use `CreateTransactionInternal`as the rest of
the wallet RPCs and it never checks against the tx-size mempool limit.
Add a check for tx-size as well as test coverage for that case.
6f8e3818af sendall: check if the maxtxfee has been exceeded (ishaanam)
Pull request description:
Previously the `sendall` RPC didn't check whether the fees of the transaction it creates exceed the set `maxtxfee`. This PR adds this check to `sendall` and a test case for it.
ACKs for top commit:
achow101:
ACK 6f8e3818af
Xekyo:
ACK 6f8e3818af
glozow:
Concept ACK 6f8e3818af. The high feerate is unlikely but sendall should respect the existing wallet options.
Tree-SHA512: 6ef0961937091293d49be16f17e4451cff3159d901c0c7c6e508883999dfe0c20ed4d7126bf74bfea8150d4c1eef961a45f0c28ef64562e6cb817fede2319f1a
There are many cases where we assume message processing is
single-threaded in order for how we access node-related memory to be
safe. Add an explicit mutex that we can use to document this, which allows
the compiler to catch any cases where we try to access that memory from
other threads and break that assumption.
b00fc44ca5 test: add coverage for 'add_inputs' dynamic default value (furszy)
ddbcfdf3d0 RPC: bugfix, 'add_inputs' default value is true unless 'inputs' are provided (furszy)
Pull request description:
This bugfix was meant to be in #25685, but decoupled it to try to make it part of 24.0 release.
It's a truly misleading functionality.
This PR doesn't change behavior in any way. Just fixes two invalid RPC help messages and adds test
coverage for the current behavior.
#### Description
In both RPC commands `send()` and `walletcreatefundedpsbt` the help message says
that `add_inputs` default value is false when it's actually dynamically set by the following statement:
```c++
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
```
Which means that, by default, `add_inputs` is true unless there is any pre-set input, in which
case, the default is false.
ACKs for top commit:
achow101:
ACK b00fc44ca5
S3RK:
ACK b00fc44ca5
Tree-SHA512: 5c68a40d81c994e0ab6de0817db69c4d3dea3a9a64a60362531bf583b7a4c37d524b740905a3f3a89cdbf221913ff5b504746625adb8622788aea93a35bbcd40
6725030e41 qt: Update translation source file for string freeze (round 2) (Hennadii Stepanov)
Pull request description:
On the day of [translation string freeze](https://github.com/bitcoin/bitcoin/issues/24987#issuecomment-1234026641), it happened that bitcoin-core/gui#660 did not include new strings from bitcoin/bitcoin#19602.
This PR includes all recent updates.
As a Transifex translator, I believe it is enough time for all translators to handle a few new strings by a release date. Also a Transifex check failure has been [fixed](bitcoin-core/gui/pull/664).
ACKs for top commit:
jarolrod:
ACK 6725030e41
Tree-SHA512: d57b841e87e389d31ec4ae9067b83f7f209e168399bc088c3234c2c66b34772739cb801f04b5038d55de115083d022d603bc976374bfd537b8ea10c10a545183
Covered cases for send() and walletcreatefundedpsbt() RPC commands:
1. Default add_inputs value with no preset inputs (add_inputs=true):
Expect: automatically add coins from the wallet to the tx.
2. Default add_inputs value with preset inputs (add_inputs=false):
Expect: disallow automatic coin selection.
3. Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount).
Expect: include inputs from the wallet.
4. Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount).
Expect: only preset inputs are used.
5. Explicit add_inputs=true, no preset inputs (same as (1) but with an explicit set):
Expect: include inputs from the wallet.
8ed2b72767 qt: Prevent wrong handling of `%2` token by Transifex (Hennadii Stepanov)
Pull request description:
On master (124e75a41e), Transifex translation check fails for 124e75a41e/src/qt/forms/intro.ui (L206) with a message:
> The expression '%2G' is not present in the translation.
In "Organization Settings" --> ["Translation checks"](https://www.transifex.com/bitcoin/settings/validations/) I have changed the status of the "**Variable substitution specifiers (like "%s") are preserved in the translations.**" check from "error" to "warning" temporarily. This setting should be reverted after applying this PR change.
[Noted](https://www.transifex.com/bitcoin/bitcoin/translate/#ru/qt-translation-024x/436102928/) by Transifex user [AHOHNMYC](https://www.transifex.com/user/profile/AHOHNMYC/).
I faced the same issue while working on Ukrainian translation.
ACKs for top commit:
katesalazar:
ACK 8ed2b72767
jarolrod:
ACK 8ed2b72767
Tree-SHA512: 304f795ac9241ac8453c614ed18d967226d9d515f9ea079b51af5bcbe2f0760ca7dcaea5efb38207720cb7a18159c2bcd337b961bc522a128715c70e0db81061
34a2f91055 Revert "doc: note that brew installed qt is not supported" (Hennadii Stepanov)
Pull request description:
As bitcoin/bitcoin#26056 fixes bitcoin/bitcoin#25947 it looks reasonable to revert bitcoin/bitcoin#21988.
ACKs for top commit:
fanquake:
ACK 34a2f91055 - haven't tested at all.
jarolrod:
ACK 34a2f91055
Tree-SHA512: 4470f21fb6ea32970d7572c83ba064bcbe6e3282cea79122312f8ac203a5b1617b21952db1d6e47ba5b6f605abc23f72c04c07cef7251272e22fb593ff317beb
The template parameter `typename Base = typename Mutex::UniqueLock` is
not used, so remove it. Use internally defined type `Base` to avoid
repetitions of `Mutex::UniqueLock`.
0f0cc05e4c refactor: Remove trailing semicolon from LOCK2 (Aurèle Oulès)
Pull request description:
Macros should not have a trailing semi-colon to avoid empty statements when using them with another semi-colon.
Noticed this while reviewing a PR.
ACKs for top commit:
vasild:
ACK 0f0cc05e4c
Tree-SHA512: 97fa4d89f5131ac30e05b293f750b757d5526feed56885c6feeb403b3ac3d3d3205874bc507c3b56a8296a6e3bdc8d879b2c339784f1e6ab1963d1b8a8d7b02f
Since it is now a string_view instead of a const char*, update the
name to reflect that the variable is no longer a "Pointer to
String, Zero-terminated" (psz).
-BEGIN VERIFY SCRIPT-
sed -i s/pszThread/thread_name/ $(git grep -l pszThread src)
-END VERIFY SCRIPT-
Rather than including validation.h, which ultimately means needing boost
via txmempool.h, include primitives/block.h for CBlock, and remove
validation.h, as we can get cs_main from node/blockstorage.h.
The only reason BOOST_CPPFLAGS is needed here, is because of the
policy/rbf.h include, which ultimately includes boost multi_index
via txmempool.h. However this include is actually unused.
In both RPC commands `send()` and `walletcreatefundedpsbt` the RPC help was saying
that `add_inputs` default value was false when it's actually dynamically set
by the following statement:
`coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;`
Which means that, by default, `add_inputs` is true unless there
was any pre-set input, in which case, the default is false.
in TestingSetup(). This is used in the following commit to test
reinitializing chainstates after snapshot validation and cleanup.
Best reviewed with `git diff --color-moved=dimmed-zebra`.
This CreateAndActivateUTXOSnapshot parameter is necessary once we
perform snapshot completion within ABC, since the existing UpdateTip
test will fail because the IBD chain that has generated the snapshot
will exceed the base of the snapshot.
Being able to test snapshots being loaded into a mostly-uninitialized
datadir allows for more realistic unittest scenarios.
