| Age | Commit message (Collapse) | Author |
|---|
|
simplify/constness
b36e0cd1b9d361ac6f9777c09328a13e9ee923be rpc: simplify addpeeraddress and improve code constness (Jon Atack)
6b1926cf1eac1ad1850599d2753dd22bc21fd327 test: addpeeraddress functional test coverage (Jon Atack)
Pull request description:
- Add functional test coverage for rpc addpeeraddress
- Simplify addpeeraddress and improve code constness
ACKs for top commit:
klementtan:
ACK [`b36e0cd`](https://github.com/bitcoin/bitcoin/pull/22043/commits/b36e0cd1b9d361ac6f9777c09328a13e9ee923be)
MarcoFalke:
review ACK b36e0cd1b9d361ac6f9777c09328a13e9ee923be 💭
Tree-SHA512: 01773fb70f23db5abf46806bb27804e48feff27272b2e6582bd5b886e9715088eb2d84755106bce2ad6f88e21582f7f071a30a89d5b17286d899c3dd8553b4fc
|
|
4f504f826bcbb1a4aa4701e87cb68da4ca05b857 rpc: fix code comment for bumpfee/psbtbumpfee output (Jon Atack)
5cb7ac23fb97a0cbc75b7eef0951da0e0bc5292b rpc: fix docs for bumpfee psbt update (Jon Atack)
Pull request description:
Follow-up to #21544 and #20891 for the `bumpfee_helper` used for RPCs bumpfee and psbtbumpfee:
- "psbt" field is only returned in psbtbumpfee and not bumpfee
- bumpfee raises if private keys are disabled, so the txid help "Only returned when wallet private keys are enabled." no longer makes sense; remove it
- add missing space in RPC examples ("Bump the fee, get the new transaction'stxid")
- update txid/psbt code comments
ACKs for top commit:
klementtan:
ACK [`4f504f8`](https://github.com/bitcoin/bitcoin/pull/22021/commits/4f504f826bcbb1a4aa4701e87cb68da4ca05b857)
Tree-SHA512: 194faf8af52383eb8ac5cd22825265931bcde135dac79d8ecc4f84f698070da9b9373c00eef8623961881bb293157c7c9a0d71d1bcccf481ae3605a2d1444ed8
|
|
a7a43e8fe85f6247c35d7ff99f36448574f3e34a Factor feefilter logic out (amadeuszpawlik)
c0385f10a133d5d8a4c296e7b7a6d75c9c4eec12 Remove -feefilter option (amadeuszpawlik)
Pull request description:
net: Remove -feefilter option, as it is debug only and isn't used in any tests. Checking this option for every peer on every iteration of the message handler is unnecessary, as described in #21545.
refactor: Move feefilter logic out into a separate `MaybeSendFeefilter(...)` function to improve readability of the already long `SendMessages(...)`. fixes #21545
The configuration option `-feefilter` has been added in 9e072a6e66efbda7d39bf61eded21d2b324323be: _"Implement "feefilter" P2P message"_
According to the [BIP133](https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki), turning the fee filter off was ment for:
> [...] a node [...] using prioritisetransaction to accept transactions whose actual fee rates might fall below the node's mempool min fee [in order to] disable the fee filter to make sure it is exposed to all possible txid's
`-feefilter` was subsequently set as debug only in #8150, with the motivation that the help message was too difficult to translate.
ACKs for top commit:
jnewbery:
Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a
promag:
Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a.
MarcoFalke:
review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a 🦁
Tree-SHA512: 8ef9a2f255597c0279d3047dcc968fd30fb7402e981b69206d08eed452c705ed568c24e646e98d06eac118eddd09205b584f45611d1c874abf38f48b08b67630
|
|
output mode
6cebac598e5e85eadd60eb1274d7f33d63ce1108 test: MiniWallet: introduce enum type for output mode (Sebastian Falbesoner)
Pull request description:
This is a follow-up PR to #21945 which lifted the number of MiniWallet's tx output modes from 2 to 3 (by adding P2PK Support).
Since the current way of specifying the mode on the ctor via two booleans is ugly and error-prone (see table in comment https://github.com/bitcoin/bitcoin/pull/21945#issuecomment-842526575), a new Enum type `MiniWalletMode` is introduced that can hold the following values:
- ADDRESS_OP_TRUE
- RAW_OP_TRUE
- RAW_P2PK
Also adds documentation that should guide the user on which mode is useful for what etc. with a summary table. (Can also be split up in a separate commit or shortened if that is desired, maybe it's considered to be too verbose).
ACKs for top commit:
MarcoFalke:
cr ACK 6cebac598e5e85eadd60eb1274d7f33d63ce1108
Tree-SHA512: cbbc10806d9d9e62829548094415e9f1a281cd247b9a9fc7f7f33b923c723aa03e7bc3024623a77fb1f7da4d73455fa8244840f746980d32acdad97ee12100da
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Easy to verify with `git diff --color-moved=dimmed-zebra`.
|
|
|
|
|
|
|
|
Only allow "packages" with no conflicts, sorted in order of dependency,
and no more than 25 for now. Note that these groups of transactions
don't necessarily need to adhere to some strict definition of a package
or have any dependency relationships. Clients are free to pass in a
batch of 25 unrelated transactions if they want to.
|
|
For the MiniWallet constructor, the two boolean parameters
"raw_script" and "use_p2pk" are replaced by a single parameter of the
newly introduced type MiniWalletMode (derived by enum.Enum), which can
hold the following values:
- ADDRESS_OP_TRUE
- RAW_OP_TRUE
- RAW_P2PK
|
|
Maximum number of transactions allowed in a package is 25, equal to the
default mempool descendant limit: if a package has more transactions
than this, either it would fail default mempool descendant limit or the
transactions don't all have a dependency relationship (but then they
shouldn't be in a package together). Same rationale for 101KvB virtual
size package limit.
Note that these policies are only used in test accepts so far.
|
|
|
|
Key functionality = a transaction with UTXOs not present in UTXO set
or mempool can be fully validated instead of being considered an orphan.
|
|
This allows us to easily create transaction chains for package
validation. We don't test_accept if submit=false because we want to be
able to make transactions that wouldn't pass ATMP (i.e. a child
transaction in a package would fail due to missing inputs).
|
|
Only allow test accepts for now. Use the CoinsViewTemporary to keep
track of coins created by each transaction so that subsequent
transactions can spend them. Uncache all coins since we only
ever do test accepts (Note this is different from ATMP which doesn't
uncache for valid test_accepts) to minimize impact on the coins cache.
Require that the input txns have no conflicts and be ordered
topologically. This commit isn't able to detect unsorted packages.
|
|
osx+win
ee883201cf134952284632e9e9ae72bf1c8c792f guix: repro: Sort find output in libtool for gcc-8 (Carl Dong)
ee0a67c32a8861eab650bf8894af06807578eba0 codesigning: Use SHA256 as digest for osslsigncode (Windows) (Carl Dong)
38eb91eb0616ed6dbe34c23e11588d130fef07f8 guix: Add codesigning functionality (Carl Dong)
bac2690e6f683fcedb883fe1d32f3c33c628a141 guix: Package codesigning tools (Carl Dong)
0a2176d47767972e4cd5ed302c1dbeedece1708b guix: Reindent existing manifest.scm (Carl Dong)
c090a3e9238ba2df07875b4708e908d8dca4ed9b Makefile.am: use APP_DIST_DIR instead of hard-coding dist (Carl Dong)
Pull request description:
This is the last PR before we reach feature-parity with the Gitian process!
Note: I tried using the `Makefile` inside the distsrc to make the dmg instead of manually listing out the commands, but `make` seems to want to re-make a lot of other files which broke the dmg.
The workflow looks something like this:
1. `env [ FOO=bar... ] ./contrib/guix/guix-build` (add additional env vars as necessary)
2. Codesigners only:
1. Copy `guix-build-<short-id>/output/x86_64-apple-darwin18/bitcoin-<short-id>-osx-unsigned.tar.gz` and `guix-build-<short-id>/output/x86_64-w64-mingw32/bitcoin-<short-id>-win-unsigned.tar.gz` to signing computer
2. Codesign with `./detached-sig-create.sh` inside the tarball
3. Upload contents of `signature-{osx,win}.tar.gz` to https://github.com/bitcoin-core/bitcoin-detached-sigs (as a new tag)
3. Checkout new tag for `bitcoin-core/bitcoin-detached-sigs` with the detached signatures
4. `env [ FOO=bar... ] DETACHED_SIGS_REPO=<path/to/bitcoin-detached-sigs> ./contrib/guix/guix-codesign` (modify env vars as necessary)
5. Make sure `guix.sigs` is cloned and updated
6. `env GUIX_SIGS_REPO=<path/to/guix.sigs> SIGNER=0x96AB007F1A7ED999=dongcarl ./contrib/guix/guix-attest` (modify env vars as necessary)
7. Commit your new signatures and SHA256SUMS in `guix.sigs`
8. Optionally, after there are multiple signatures in `guix.sigs`: `env GUIX_SIGS_REPO=<path/to/guix.sigs> ./contrib/guix/guix-verify`
ACKs for top commit:
laanwj:
Tested ACK ee883201cf134952284632e9e9ae72bf1c8c792f
achow101:
ACK ee883201cf134952284632e9e9ae72bf1c8c792f
Tree-SHA512: e812a07a5f19f900600c70cb9c717769ef544a6c0c12760b5558b76b6b37df863257f3dbf38b0757e6e06e334470267e94c9f2bdbc27409d6837b1a0bfc6acbc
|
|
Break SendMessages() function into smaller units to improve readability.
Adds lock assert, as `round()` isn't thread safe.
closes #21545
|
|
skipping DNS seed
fe3d17df04decc4e856121eb311636977d60f80f net: ignore block-relay-only peers when skipping DNS seed (Anthony Towns)
Pull request description:
Since #17428 bitcoind will attempt to reconnect to two block-relay-only anchors before doing any other outbound connections. When determining whether to use DNS seeds, it will currently see these two peers and decide "we're connected to the p2p network, so no need to lookup DNS" -- but block-relay-only peers don't do address relay, so if your address book is full of invalid addresses (apart from your anchors) this behaviour will prevent you from recovering from that situation.
This patch changes it so that it only skips use of DNS seeds when there are two full-outbound peers, not just block-relay-only peers.
ACKs for top commit:
Sjors:
utACK fe3d17d
amitiuttarwar:
ACK fe3d17df04decc4e856121eb311636977d60f80f, this impacts the very common case where we stop/start a node, persisting anchors & have a non-empty addrman (although, to be clear, wouldn't be particularly problematic in the common cases where the addrman has valid addresses)
mzumsande:
ACK fe3d17df04decc4e856121eb311636977d60f80f
jonatack:
ACK fe3d17df04decc4e856121eb311636977d60f80f
prayank23:
tACK https://github.com/bitcoin/bitcoin/pull/22013/commits/fe3d17df04decc4e856121eb311636977d60f80f
Tree-SHA512: 9814b0d84321d7f45b5013eb40c420a0dd93bf9430f5ef12dce50d1912a18d5de2070d890a8c6fe737a3329b31059b823bc660b432d5ba21f02881dc1d951e94
|
|
net_processing
0829516d1f3868c1c2ba507feee718325d81e329 [refactor] Remove unused ForEachNodeThen() template (John Newbery)
09cc66c00e1d5fabe11ffcc32cad060e6b483b20 scripted-diff: rename address relay fields (John Newbery)
76568a3351418c878d30ba0373cf76988f93f90e [net processing] Move addr relay data and logic into net processing (John Newbery)
caba7ae8a505a4b4680a9d7618f65c4e8579a1e2 [net processing] Make RelayAddress() a member function of PeerManagerImpl (John Newbery)
86acc9646968213aaa4408635915b1bfd75a10c9 [net processing] Take NodeId instead of CNode* as originator for RelayAddress() (John Newbery)
Pull request description:
This continues the work of moving application layer data into net_processing, by moving all addr data into the new Peer object added in #19607.
For motivation, see #19398.
ACKs for top commit:
laanwj:
Code review ACK 0829516d1f3868c1c2ba507feee718325d81e329
mzumsande:
ACK 0829516d1f3868c1c2ba507feee718325d81e329, reviewed the code and ran tests.
sipa:
utACK 0829516d1f3868c1c2ba507feee718325d81e329
hebasto:
re-ACK 0829516d1f3868c1c2ba507feee718325d81e329
Tree-SHA512: efe0410fac288637f203eb37d1999910791e345872d37e1bd5cde50e25bb3cb1c369ab86b3a166ffd5e06ee72e4508aa2c46d658be6a54e20b4f220d2f57d0a6
|
|
|
|
architecture-independent
fafd121026c4f1e25d498983e4f88c119516552b refactor: Make CFeeRate constructor architecture-independent (MarcoFalke)
Pull request description:
Currently the constructor is architecture dependent. This is confusing for several reasons:
* It is impossible to create a transaction larger than the max value of `uint32_t`, so a 64-bit `size_t` is not needed
* Policy (and consensus) code should be arch-independent
* The current code will print spurious compile errors when compiled on 32-bit systems:
```
policy/feerate.cpp:23:22: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
```
Fix all issues by making it arch-independent. Also, fix `{}` style according to dev notes.
ACKs for top commit:
theStack:
re-ACK fafd121026c4f1e25d498983e4f88c119516552b
promag:
Code review ACK fafd121026c4f1e25d498983e4f88c119516552b.
Tree-SHA512: e16f75bad9ee8088b87e873906d9b5633449417a6996a226a2f37d33a2b7d4f2fd91df68998a77e52163de20b40c57fadabe7fe3502e599cbb98494178591833
|
|
aca0e5dcdb174ef7e88b76910d6fffd633688235 Remove `GetDataDir(bool fNetSpecific = true)` function (Kiminuo)
b3e67f20a02bcf43f20873029ce30617c95eb9da scripted-diff: Replace `GetDataDir(true)` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
4c3a5dcbfc3010965332ad568c3a70618c930ef3 scripted-diff: Replace `GetDataDir()` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
13bd8bb0536f008118b1de921654925ed9ce1da7 Make `ArgsManager.GetDataDirPath` private and drop needless suffix (Kiminuo)
4d8189f62076d47be182e709857774bc3921b334 scripted-diff: Change `ArgsManager.GetDataDirPath()` to `ArgsManager.GetDataDirBase()` in tests (Kiminuo)
0f53df47d5b8319fc71f3b87c15a115ff797fb50 Add `ArgsManager.GetDataDirBase()` and `ArgsManager.GetDataDirNet()` as an intended replacement for `ArgsManager.GetDataDirPath(net_identifier)` (Kiminuo)
716de29dd8672d28bfe0a08424e9958fe61054d2 Make `m_cached_blocks_path` mutable. Make `ArgsManager::GetBlocksDirPath()` const. (Kiminuo)
Pull request description:
This PR is a follow up PR to #21244. The PR attempts to move us an inch towards the [goal](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615307465) by removing `GetDataDir(net_specific)` and replacing it by `gArgs.GetDataDir(net_specific)` calls.
The approach of this PR attempts to be similar to the one chosen in "De-globalize ChainstateManager" (#20158). The goal is to pass `ArgsManager` to functions (or ideally to have `ArgsManager` as a member of a class where needed; inspiration from here: #21789) instead of having it as a global variable (i.e. `gArgs`).
**Notes:**
* First commit makes `m_cached_blocks_path` `mutable` as was suggested [here](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615274095) but not fully applied in #21244. (`m_cached_datadir_path` and `m_cached_network_datadir_path` were marked as `mutable` in #21244) This commit can be in a separate PR too.
* Other commits deal with removing of `GetDataDir(net_specific)` function.
* This was originally part of #21244 but it was [left]((https://github.com/bitcoin/bitcoin/pull/21244#pullrequestreview-633779754)) for a follow up PR.
* I think that the proposed changes show nicely where there is reliance on `gArgs` which is IMO a good thing.
If you know about a better approach how to do this, please share it here.
ACKs for top commit:
hebasto:
ACK aca0e5dcdb174ef7e88b76910d6fffd633688235
MarcoFalke:
re-ACK aca0e5dcdb174ef7e88b76910d6fffd633688235 👃
Tree-SHA512: deec4d88edb32d7f4c818c3a74ffbb64709685819b88242dcf5dbaa1fb611f3ce2b29d2576ddb9e0dc5e75288e43538968224008c0a80e7149fc81c309f7c9da
|
|
|
|
calls
-BEGIN VERIFY SCRIPT-
git ls-files -- 'src' ':(exclude)src/util/system.h' ':(exclude)src/util/system.cpp' | xargs sed -i 's/GetDataDir(true)/gArgs.GetDataDirNet()/g';
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
git ls-files -- 'src' ':(exclude)src/util/system.h' ':(exclude)src/util/system.cpp' | xargs sed -i 's/GetDataDir()/gArgs.GetDataDirNet()/g';
-END VERIFY SCRIPT-
|
|
|
|
`ArgsManager.GetDataDirBase()` in tests
-BEGIN VERIFY SCRIPT-
git ls-files src/test/*_tests.cpp src/test/util/setup_common.cpp | xargs sed -i 's/.GetDataDirPath()/.GetDataDirBase()/g';
-END VERIFY SCRIPT-
|
|
intended replacement for `ArgsManager.GetDataDirPath(net_identifier)`
|
|
4bea30169218e2f21e0c93a059966b41c8edd205 test: use P2PK-MiniWallet for feature_csv_activation.py (Sebastian Falbesoner)
dc7eb64e83f5b8e63f12729d5f77b1c920b136e4 test: MiniWallet: add P2PK support (Sebastian Falbesoner)
Pull request description:
This PR adds support for creating and spending transactions with raw pubkey (P2PK) outputs to MiniWallet, [as suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/21900#discussion_r629524841). Using that mode in the test `feature_csv_activation.py`, all txs submitted to the mempool follow the standard policy, i.e. `-acceptnonstdtxn=1` can be removed.
Possible follow-ups:
* Improve MiniWallet constructor Interface; an enum-like parameter instead of two booleans would probably be better
* Look at other tests that could benefit from P2PK (e.g. feature_cltv.py?)
* Check vsize also for P2PK txs (vsize varies due to signature, i.e. a range has to be asserted)
ACKs for top commit:
laanwj:
Code review ACK 4bea30169218e2f21e0c93a059966b41c8edd205
Tree-SHA512: 9b428e6b7cfde59a8c7955d5096cea88af1384a5f49723f00052e9884d819d952d20a5ab39bb02f9d8b6073769c44462aa265d84a33e33da33c2d21670c488a6
|
|
When building with Clang, if `-fstack-clash-protection` is used with an
unsupported target, it may result in hundreds of
`-Wunused-command-line-argument` warnings at compile time. This is
currently the case when building for at least Darwin using Apple or LLVM
Clang.
Unsupported targets may also include *BSD, however that is changing; see
further discussion in https://reviews.llvm.org/D92245 and
https://reviews.freebsd.org/D27366.
Note that this option is already skipped for Windows.
|
|
-noincludeconf=0
fad0867d6ab9430070aa7d60bf7617a6508e0586 Cleanup -includeconf error message (MarcoFalke)
fa9f711c3746ca3962f15224285a453744cd45b3 Fix crash when parsing command line with -noincludeconf=0 (MarcoFalke)
Pull request description:
The error message has several issues:
* It may crash instead of cleanly shutting down, when `-noincludeconf=0` is passed
* It doesn't quote the value
* It includes an erroneous trailing `\n`
* It is redundantly mentioning `"-includeconf cannot be used from commandline;"` several times, when once should be more than sufficient
Fix all issues by:
* Replacing `get_str()` with `write()` to fix the crash and quoting issue
* Remove the `\n` and only print the first value to fix the other issues
Before:
```
$ ./src/bitcoind -noincludeconf=0
terminate called after throwing an instance of 'std::runtime_error'
what(): JSON value is not a string as expected
Aborted (core dumped)
$ ./src/bitcoind -includeconf='a b' -includeconf=c
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=a b
-includeconf cannot be used from commandline; -includeconf=c
```
After:
```
$ ./src/bitcoind -noincludeconf=0
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=true
$ ./src/bitcoind -includeconf='a b' -includeconf=c
Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf="a b"
```
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34493
Testcase: https://github.com/bitcoin/bitcoin/files/6515429/clusterfuzz-testcase-minimized-system-6328535926046720.log
```
FUZZ=system ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-system-6328535926046720.log
```
See https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md
ACKs for top commit:
sipa:
utACK fad0867d6ab9430070aa7d60bf7617a6508e0586
Tree-SHA512: b44af93be6bf71b43669058c1449c4c6999f03b5b01b429851b149b12d77733408cb207e9a3edc6f0bffd6030c4c52165e8e23a1c2718ff5082a6ba254cc94a4
|
|
|
|
|
|
QProgressDialog estimates the time the operation will take (based on
time for steps), and only shows itself if that estimate is beyond
minimumDuration. The default minimumDuration value is 4 seconds, and it
could make users think that the GUI is frozen.
|
|
QProgressDialog shows itself if the estimated time an operation will
take is beyond the minimumDuration value.
Direct call show() breaks that behavior on macos.
|
|
const.
|
|
fa91994b1b9b6da3bb2a2f722d9251d7d0f2167e fuzz: Add utxo_snapshot target (MarcoFalke)
Pull request description:
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/21953/commits/fa91994b1b9b6da3bb2a2f722d9251d7d0f2167e
Tree-SHA512: a00f077102a4e4e321bd1464c3fa11e7a5b9e04324b9be87aa28cfdc77630db7fc772d3a3768dc6ec36bbdd2d67b7e0719f0cf3fd87b4a1087365b934e137b5c
|
|
|
|
- "psbt" field is only returned in psbtbumpfee and not bumpfee
- bumpfee raises if privkeys are disabled, so drop "Only returned when wallet private keys are enabled."
- add missing space in RPC example
|
|
|
|
|
|
Remove the erroneous trailing newline '\n'. Also, print only the first
value to remove needless redundancy in the error message.
|
|
|
|
bbbb51877a96c3473aeea5914f751eec7835b5c4 fuzz: Speed up transaction fuzz target (MarcoFalke)
Pull request description:
`hashBlock` and `include_addresses` are orthogonal, so no need to do an exhaustive "search".
Might fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34491
ACKs for top commit:
practicalswift:
cr ACK bbbb51877a96c3473aeea5914f751eec7835b5c4: patch looks correct, and `TxToUniv` surprisingly wide in the `transaction_fuzz_target` flame graph! Putting it on a diet makes sense.
Tree-SHA512: 1e7c30c7fecf96364a9a1597c0a22139389fdeb67db59f3c2c6fc088196e3332877b2865991a957980d542f99a2f48cc066dd7cc16c695a5113190fe06205089
|
