Age | Commit message (Collapse) | Author |
|
|
|
|
|
4e0613369f446b0a57783bf9e7258fec6c474981 qt: Elide long strings in their middle in the Peers tab (Hennadii Stepanov)
Pull request description:
The eliding of long addresses (Onion v3 etc) in the Peers tab in their middle was [discussed](https://github.com/bitcoin-core/gui/issues/262#issuecomment-810490396) in #262.
On master (f0fa32450ec35056b3e1025262aeaef4a24c35ee):
![DeepinScreenshot_select-area_20210410141435](https://user-images.githubusercontent.com/32963518/114267903-24eea400-9a07-11eb-8c80-99f68d5cc522.png)
With this PR:
![DeepinScreenshot_select-area_20210410140430](https://user-images.githubusercontent.com/32963518/114267796-83675280-9a06-11eb-921f-ca47c2075496.png)
This PR suggests the minimal diff to achieve the goal. OTOH, this change in behavior is common for all columns in the Peers table, but it seems harmless.
ACKs for top commit:
jarolrod:
tACK 4e0613369f446b0a57783bf9e7258fec6c474981
promag:
Code review ACK 4e0613369f446b0a57783bf9e7258fec6c474981.
Tree-SHA512: 1d5a62afb1152029e69fccea2ae53dcb262a91724a5c03dfc4de8c409b280814d0c211c2f9a71f1a6e927f4ed571ba4ac311de9de8ebb797eaf1051674241bdb
|
|
b109bde46a8730afbc09c107b802a2c321293f4b [test] check that mapFlagNames is up to date (glozow)
5d3ced72f9b5f36db1a76bd8bc918d11b87dfd72 [test] remove unnecessary OP_1s from invalid tests (glozow)
5aee73d1759bcc0d1e951776942e616843934af1 [test] minor improvements / followups (glozow)
8a365df5586b36d1772c78069f9d93c56a81df6f [test] fix bug in ExcludeIndividualFlags (glozow)
8cac2923f57ac33848ff41b74c3be520b75936df [test] remove invalid test from tx_valid.json (glozow)
Pull request description:
This is a followup to #19698.
- There was a bug in the `ExcludeIndividualFlags` function which is fixed here.
- Fixing this bug also showed that there is a test that's supposed to fail (already existing in tx_invalid.json) in tx_valid.json, so I removed it. Other than that, the tests should all pass.
- Also implements a few suggestions I received offline: removing the `OP_1`s from the invalid tests (similar to https://github.com/bitcoin/bitcoin/commit/19db590d044efe7d474a16720e5b56e7b55db54c), comments, and style.
- A few other small fixes, like adding asserts, putting all the flags in `mapFlagNames`, better error messages
ACKs for top commit:
jnewbery:
utACK b109bde46a8730afbc09c107b802a2c321293f4b
Tree-SHA512: 7233a8c0f1ae1172fac8000ea6e05384ecf79074c39948d118464868505c7f02f17e96503c81bd05c07adb2087648a5d93d9899e16fdefa6b7efcb51319444a9
|
|
bee56c78e94417f89b1f48682404e2821b57bdec rpc: Check default value type againts argument type (João Barbosa)
f81ef4303e057e85aa24772c865287c17ffa4350 rpc: Keep default argument value in correct type (João Barbosa)
Pull request description:
Store default values of RPC arguments in the corresponding type instead of a string. The value is then serialized when the help output is needed. This change simplifies #20017.
The following examples illustrates how to use the new `RPCArg::Default` and `RPCArg::DefaultHint`:
```diff
- {"verbose", RPCArg::Type::BOOL, /* default */ "false", "True for a json object, false for array of transaction ids"}
+ {"verbose", RPCArg::Type::BOOL, RPCArg::Default(false), "True for a json object, false for array of transaction ids"}
```
```diff
- {"nblocks", RPCArg::Type::NUM, /* default */ "one month", "Size of the window in number of blocks"}
+ {"nblocks", RPCArg::Type::NUM, RPCArg::DefaultHint("one month"), "Size of the window in number of blocks"}
```
No behavior change is expected.
ACKs for top commit:
LarryRuane:
ACK bee56c78e94417f89b1f48682404e2821b57bdec
MarcoFalke:
ACK bee56c78e94417f89b1f48682404e2821b57bdec 🦅
Tree-SHA512: c47d78c918e996d36631d4ad3c933b270a34c5b446b8d736be94cf4a0a7b8c0e33d954149ec786cf9550639865b79deb6a130ad044de6030f95aac33f524293a
|
|
9a0653553a0ec403b4e7c6713466e0c7fa10ec94 Refactor ProcessNewBlock to reduce code duplication (R E Broadley)
Pull request description:
There are probably a few issues with this code (maybe there's even a reason this code is duplicated as it currently is), so apologies in advance that I'm still a little (maybe very) bad with C++
ACKs for top commit:
MarcoFalke:
ACK 9a0653553a0ec403b4e7c6713466e0c7fa10ec94 💻
promag:
Code review ACK 9a0653553a0ec403b4e7c6713466e0c7fa10ec94.
theStack:
Code-review ACK 9a0653553a0ec403b4e7c6713466e0c7fa10ec94 🌴
Tree-SHA512: f8634ffad4b2370204d1a0945db4e27248b9e579d9912784da432b8ee3303cae424fa9f7500000dcfb31e6d29d04a8f7d322d17a6fe3d4adaddd10c539458a8c
|
|
a41149426168b8ea96099f10576022c6a09033d1 rpc: Improve getblock error message for invalid data type. (klementtan)
Pull request description:
Improve error messages for getblock invalid datatype.
fixes: #21717
ACKs for top commit:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/21718/commits/a41149426168b8ea96099f10576022c6a09033d1
theStack:
ACK a41149426168b8ea96099f10576022c6a09033d1
promag:
Code review ACK a41149426168b8ea96099f10576022c6a09033d1.
Tree-SHA512: 6e7d8290681e8ab375629f81669d0f8e0c21f9eb7ed9e2455cd19ea013e69b2d95fa7a9ee795315b2d5c60c96035c6cefc3d6e1039a06fd88c1dc7fe275ee6a1
|
|
|
|
586190f0b4740457cb86cba632e3d64e6dfe9b0c rpc/rest: Take and reuse local Chain/ChainState obj (Carl Dong)
bc3bd369027273278a0541f3b991eb71de831aa2 rpc: style: Improve BuriedForkDescPushBack signature (Carl Dong)
f99913969f92b8b9cef1b83f5ee8e6a9267b4af0 rpc: Remove unnecessary casting of block height (Carl Dong)
6a3d1920209cded0dae52fb9070a3530d9a4e5fd rpc: Tidy up local references (see commit message) (Carl Dong)
038854f31e3511e8bb6e163305cab0a96783d25b rest/rpc: Remove now-unused old Ensure functions (Carl Dong)
6fb65b49f4d393b091479be5a5df5a0a160cf986 scripted-diff: rest/rpc: Use renamed EnsureAny*() (Carl Dong)
1570c7ee98612366df031bebef9e0468fb57b8a2 rpc: Add renamed EnsureAny*() functions (Carl Dong)
306b1cd3eeb2502904ed4698646d2c86d028aad2 rpc: Add alt Ensure* functions acepting NodeContext (Carl Dong)
d7824acdb9b18fe8f151771a83ccae1681f16c66 rest: Use existing NodeContext (Carl Dong)
3f0893479908ca28d6127c8d0ada30737cb830be rest: Pass in NodeContext to rest_block (Carl Dong)
7be0671b950842fc3a17641a4a21501de0a800b5 rpc/rawtx: Use existing NodeContext (Carl Dong)
60dc05afc6f6388c6f86729a0edd7cb69f1748e0 rpc/mining: Use existing NodeContext (Carl Dong)
d485e815e2b62dc74a485569d08130dc3ef9ff63 rpc/blockchain: Use existing NodeContext (Carl Dong)
d0abf0bf429586e3a5b4c3231fe430dc29695481 rpc/*,rest: Add review-only assertion to EnsureChainman (Carl Dong)
cced0f46c9133e0fc6211e987421ad1d9be1a399 miner: Pass in previous CBlockIndex to RegenerateCommitments (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Based on:
- [x] #21270 | [Bundle 4/n] Prune g_chainman usage in validation-adjacent modules
- [x] #21525 | [Bundle 4.5/n] Followup fixups to bundle 4
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
ryanofsky:
Code review ACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c. Since last review, no changes to existing commits, just some simple new commits added: three new commits renaming std::any Ensure functions (scripted diff commit and manual pre/post commits), and one new commit factoring out a repeated `ActiveChain()` call made in a loop. Thanks for the updates!
jnewbery:
utACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c
MarcoFalke:
review ACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c 🍯
Tree-SHA512: 64b677fb50141805b55c3f1afe68fcd298f9a071a359bdcd63256d52e334f83e462f31fb3ebee9b630da8f1d912a03a128cfc38179e7aaec29a055744a98478c
|
|
macOS.
7f3a5980c1d54988a707b961fd2ef647cebb4c5b qt: Do not use QClipboard::Selection on Windows and macOS. (Hennadii Stepanov)
Pull request description:
Windows and macOS do [not support](https://doc.qt.io/qt-5/qclipboard.html#notes-for-windows-and-macos-users) the global mouse selection.
Fixes #258.
ACKs for top commit:
promag:
Code review ACK 7f3a5980c1d54988a707b961fd2ef647cebb4c5b.
jarolrod:
ACK 7f3a5980c1d54988a707b961fd2ef647cebb4c5b
Tree-SHA512: be2beeef7d25af6f4d4a4548325d8d29f08e4342f499666bc4a670ed468a63195d514077c2cd0dba197e12bd43316fd3e2813cdc0954364b6aa4ae6b90c118bf
|
|
`BOOST_CHECK` in `cnetaddr_basic`
63631beef6a0046390469971adf4500718ab34ad test: Remove intermittently failing and not very meaningful `BOOST_CHECK` in `cnetaddr_basic` (practicalswift)
Pull request description:
Remove intermittently failing and not very meaningful `BOOST_CHECK` in `cnetaddr_basic`.
Fixes #21682.
Rationale from https://github.com/bitcoin/bitcoin/issues/21682#issuecomment-819897122:
> I've looked at that test before and I don't think that specific `BOOST_CHECK` makes much sense TBH :)
>
> 1.) I don't understand why we test if `ToString()` output includes `%zone_index`: it clearly doesn't on some platforms, so we cannot rely on it anyways. Then why test it?
>
> 2.) And perhaps more fundamentally: why would we even _want_ to have `%zone_index` in our textual `ToString()` output? I think the expectation is to get say `fe80::1ff:fe23:4567:890a` (without zone index) and not say `fe80::1ff:fe23:4567:890a%eth2 ` or `fe80::1ff:fe23:4567:890a%3 `when doing `ipv6_addr.ToString()` :)
ACKs for top commit:
MarcoFalke:
review ACK 63631beef6a0046390469971adf4500718ab34ad
Tree-SHA512: 06863d1edfb9ad1ca9bcae09cf3f0f47b58bb29d222b70799c3dc059b96452889026e4b99b132782846d9896e3e798d17c7f9406e0e6a0bec1bffc6edb54e9df
|
|
|
|
|
|
|
|
549c82ad3a34a885ecca37a5f04c36dfbaa95d17 fuzz: use ConsumeBool() instead of !ConsumeBool() (Vasil Dimov)
29ae1c13a59187119f5b2a38b54dbbec936d8f87 fuzz: split FuzzedSock interface and implementation (Vasil Dimov)
9668e43d8e757c0185b900eb6ee6891a0ba41666 fuzz: make FuzzedSock::Wait() sometimes simulate an occurred event (Vasil Dimov)
0c90ff1429deaa556c0509c13cdd5aef5df9c0d4 fuzz: set errno from FuzzedSock::Wait() if it simulates a failure (Vasil Dimov)
5198a02de4e7a1b0efe28c6095745ce59f7f98c4 style: remove extra white space (Vasil Dimov)
Pull request description:
* split FuzzedSock interface and implementation
* make FuzzedSock::Wait() sometimes simulate an occurred event
* set errno from FuzzedSock::Wait() if it simulates a failure
(this is a followup from https://github.com/bitcoin/bitcoin/pull/21617)
ACKs for top commit:
practicalswift:
cr ACK 549c82ad3a34a885ecca37a5f04c36dfbaa95d17: patch looks correct and touches only `src/test/fuzz/`
MarcoFalke:
re-ACK 549c82ad3a34a885ecca37a5f04c36dfbaa95d17 only change is rebase 🎬
Tree-SHA512: 8ba965a8319074ad2ef840219c35c77e37cc79f00fb3926f20ccbf5f58e9616f5a3ac96434ad33996b47d292fa760d5d00a529001ac0d1d254262e5df93f616f
|
|
f979b3237f1cfc28f9c4ccb07beab558d5357a55 Add mainnet and testnet taproot activation params (Andrew Chow)
Pull request description:
Adds the activation parameters for taproot as specified in https://github.com/bitcoin/bips/pull/1104
ACKs for top commit:
gmaxwell:
utACK f979b3237f1cfc28f9c4ccb07beab558d5357a55
ajtowns:
ACK f979b3237f1cfc28f9c4ccb07beab558d5357a55
instagibbs:
ACK f979b32
clarkmoody:
ACK f979b32
Sjors:
ACK f979b3237f1cfc28f9c4ccb07beab558d5357a55
jonatack:
utACK f979b3237f1cfc28f9c4ccb07beab558d5357a55 verified with the BIP draft
Tree-SHA512: f95538bcec46c36f9532a99fcf697b143083c25b2427dd578b88514add0a807371530c18f0a8ed040dc885ad6eca8234235e1d762f6f837eafc5daed856a9dcf
|
|
fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 test: Reset mocktime in the common setup (MarcoFalke)
fa78590a8fffdfc7e98ddb1f81218f05b1935a0a test: Use mocktime to avoid intermittent failure (MarcoFalke)
Pull request description:
See https://github.com/bitcoin/bitcoin/pull/21602#discussion_r611176103
ACKs for top commit:
jonatack:
Code review ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1
jarolrod:
ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1
Tree-SHA512: 4967e006f3d2c4eb92f03c9086a6abe3190ad54755d251c30d20422c574bb1a154c06f3d5bcb0d4deaa3c4abfd3864d743b71d84897edd358e829bb42233ad12
|
|
The former is shorter and ends up with a "random" bool anyway.
|
|
Move the `FuzzedSock`'s implementation from `src/test/fuzz/util.h` to
`src/test/fuzz/util.cpp`.
A separate interface and implementation make the code more readable for
consumers who don't need to (better not) know the implementation
details.
|
|
|
|
|
|
|
|
`cnetaddr_basic`
|
|
use) in FuzzedSock
6262182b3f1c9540291fb8de3bf7a785e7113c55 Avoid use of low file descriptor ids (which may be in use) in FuzzedSock and StaticContentsSock (practicalswift)
Pull request description:
Avoid use of low file descriptor ids (which may be in use) in `FuzzedSock`.
Context: https://github.com/bitcoin/bitcoin/pull/21630/files#r610694541
ACKs for top commit:
vasild:
ACK 6262182b3f1c9540291fb8de3bf7a785e7113c55
Tree-SHA512: e622acb4d01446c3db01adbbbb779038be7247e13f3f4e72c614bc2880c3efd710fd3b189f87abb00f236fa5ddf91f4c215f420ca4eb08a97aaba31593254c3d
|
|
|
|
ffe33dfbd4c3b11e3475b022b6c1dd077613de79 chainparams: drop versionbits threshold to 90% for mainnnet and signet (Anthony Towns)
f054f6bcd2c2ce5fea84cf8681013f85a444e7ea versionbits: simplify state transitions (Anthony Towns)
55ac5f568a3b73d6f1ef4654617fb76e8bcbccdf versionbits: Add explicit NEVER_ACTIVE deployments (Anthony Towns)
dd07e6da48040dc7eae46bc7941db48d98a669fd fuzz: test versionbits delayed activation (Anthony Towns)
dd85d5411c1702c8ae259610fe55050ba212e21e tests: test versionbits delayed activation (Anthony Towns)
73d4a706393e6dbd6b6d6b6428f8d3233ac0a2d8 versionbits: Add support for delayed activation (Anthony Towns)
9e6b65f6fa205eee5c3b99343988adcb8d320460 tests: clean up versionbits test (Anthony Towns)
593274445004506c921d5d851361aefb3434d744 tests: test ComputeBlockVersion for all deployments (Anthony Towns)
63879f0a4760c0c0f784029849cb5d21ee088abb tests: pull ComputeBlockVersion test into its own function (Anthony Towns)
Pull request description:
BIP9-based implementation of "speedy trial" activation specification, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-March/018583.html
Edge cases are tested by fuzzing added in #21380.
ACKs for top commit:
instagibbs:
tACK https://github.com/bitcoin/bitcoin/pull/21377/commits/ffe33dfbd4c3b11e3475b022b6c1dd077613de79
jnewbery:
utACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79
MarcoFalke:
review ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 💈
achow101:
re-ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79
gmaxwell:
ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79
benthecarman:
ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79
Sjors:
ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79
jonatack:
Initial approach ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 after a first pass of review, building and testing each commit, mostly looking at the changes and diffs. Will do a more high-level review iteration. A few minor comments follow to pick/choose/ignore.
ariard:
Code Review ACK ffe33df
Tree-SHA512: f79a7146b2450057ee92155cbbbcec12cd64334236d9239c6bd7d31b32eec145a9781c320f178da7b44ababdb8808b84d9d22a40e0851e229ba6d224e3be747c
|
|
StaticContentsSock
|
|
Doing it there will reduce code bloat and also ensure no test can "forget" to reset it
|
|
In all rest/rpc-related modules, if there are multiple calls to
ActiveChain{,State}(), and the calls fall under the same ::cs_main lock,
we can simply take a local reference and use/reuse it instead of calling
ActiveChain{,State}() again and again.
|
|
|
|
|
|
Organize local variables/references such that:
1. There is always a `ChainstateManager` reference before any `LOCK(cs_main)`.
2. NodeContext references are used with Ensure*() functions introduced in
previous commit where appropriate to avoid duplicate assertions.
|
|
The scripted-diff in the previous commit should have removed all calls
to functions like: Ensure(?!Any)\(const std::any& (context|ctx)\), so we
can remove them now.
|
|
-BEGIN VERIFY SCRIPT-
sed -i -E 's@Ensure([^(]+)(\((request\.|)context\))@EnsureAny\1\2@g' \
-- src/rest.cpp src/rpc/*.cpp
-END VERIFY SCRIPT-
|
|
- The original Ensure*(const std::any& context) functions are kept and
the parameter renamed to ctx so that the scripted-diff in the
subsequent commit will work as expected
- The renaming avoids overloading mistakes arising out of the untyped
std::any argument.
|
|
fa73ce6e653d00824eb68f772fd29b7f8fb93d84 Fix assumeutxo crash due to truncated file (MarcoFalke)
Pull request description:
ACKs for top commit:
jamesob:
ACK fa73ce6e653d00824eb68f772fd29b7f8fb93d84
ryanofsky:
Code review ACK fa73ce6e653d00824eb68f772fd29b7f8fb93d84. Easy fix. It seems like this could have been caught in review, though.
Tree-SHA512: 3a98687c386e3995114ddf0ad7194fadd9520989290681ef703b578e3ca21aee51eadfb83aa38a489bac13d12709ea137b9b184b08e5bfa2919cca177aab90be
|
|
b8e5d0d3fe3386807d47f50d13ac34fcd2a538fd qt: Handle exceptions in SendCoinsDialog::sendButtonClicked slot (Hennadii Stepanov)
1ac2bc7ac070dfd1df1872d759540b0c92495301 qt: Handle exceptions in TransactionView::bumpFee slot (Hennadii Stepanov)
bc00e13bc800863641b3e1e64732a38418d3022f qt: Handle exceptions in WalletModel::pollBalanceChanged slot (Hennadii Stepanov)
eb6156ba1b4c303eb597e3fc4a9e42ce45e6e78d qt: Handle exceptions in BitcoinGUI::addWallet slot (Hennadii Stepanov)
f7e260a471010e2d656fbc5ea8c310f6d94c26b9 qt: Add GUIUtil::ExceptionSafeConnect function (Hennadii Stepanov)
64a8755af396f1c2791018510e22b58114e68594 qt: Add BitcoinApplication::handleNonFatalException function (Hennadii Stepanov)
af7e365b1516d660d271475fdfe0c20ae09e66a8 qt: Make PACKAGE_BUGREPORT link clickable (Hennadii Stepanov)
Pull request description:
This PR is an alternative to https://github.com/bitcoin/bitcoin/pull/18897, and is based on Russ' [idea](https://github.com/bitcoin/bitcoin/pull/18897#pullrequestreview-418703664):
> IMO it would be nice to have a followup PR that eliminated the one-line forwarding methods ...
Related issues
- #91
- https://github.com/bitcoin/bitcoin/issues/18643
Qt docs: https://doc.qt.io/qt-5.12/exceptionsafety.html#exceptions-in-client-code
With this PR the GUI handles the wallet-related exception, and:
- display it to a user:
![Screenshot from 2021-04-01 02-55-59](https://user-images.githubusercontent.com/32963518/113226183-33ff8480-9298-11eb-8fe6-2168834ab09a.png)
- prints a message to `stderr`:
```
************************
EXCEPTION: 18NonFatalCheckError
wallet/wallet.cpp:2677 (IsCurrentForAntiFeeSniping)
Internal bug detected: '!chain.findBlock(block_hash, FoundBlock().time(block_time))'
You may report this issue here: https://github.com/bitcoin/bitcoin/issues
bitcoin in QPushButton->SendCoinsDialog
```
- writes a message to the `debug.log`
- and, if the exception is a non-fatal error, leaves the main window running.
ACKs for top commit:
laanwj:
Code review ACK b8e5d0d3fe3386807d47f50d13ac34fcd2a538fd
ryanofsky:
Code review ACK b8e5d0d3fe3386807d47f50d13ac34fcd2a538fd. This is great! I think more improvements are possible but implementation is very clean and I love how targeted each commit is. Changes since last review: adding more explanatory text, making links clickable, reorganizing.
Tree-SHA512: a9f2a2ee8e64b993b0dbc454edcbc39c68c8852abb5dc1feb58f601c0e0e8014dca81c72733aa3fb07b619c6f49b823ed20c7d79cc92088a3abe040ed2149727
|
|
|
|
c8f469c6d50a8db6d92f0aed47a5d1cc82f30f7f external_signer: remove ExternalSignerException (fanquake)
9e0b199b976617edeb1c58d4203df5f83a26c1e3 external_signer: use const where appropriate (fanquake)
aaa4e5a45bd9ec5563ffa7b9e0d46d2de3cb9242 wallet: remove CWallet::GetExternalSigner() (fanquake)
06a0673351282fff1673f3679a7cad9a7faaf987 external_signer: remove ignore_errors from Enumerate() (fanquake)
8fdbb899b84a2be85e632e45f08b222db02395d9 refactor: unify external wallet runtime errors (fanquake)
f4652bf1259d5c52ff0d500c732f40ba41256817 refactor: add missing includes to external signer code (fanquake)
54569cc6d6f54788169061004026e62e1c08440e refactor: move all signer code inside ENABLE_EXTERNAL_SIGNER #ifdefs (fanquake)
Pull request description:
These are a few followups after #21467.
ACKs for top commit:
Sjors:
tACK c8f469c6d50a8db6d92f0aed47a5d1cc82f30f7f
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/21666/commits/c8f469c6d50a8db6d92f0aed47a5d1cc82f30f7f
Tree-SHA512: 3d5ac5df81680075e71e0e4a7595c520d746c3e37f016cf168c1e10da15541ebb1595aecaf2c08575636e9ff77d499644cae53180232b7049cfae0b923106e4e
|
|
41f891da508114f1fd4df30b4068073ec30abc2a tests: Skip SQLite fsyncs while testing (Andrew Chow)
Pull request description:
Since we want tests to run quickly, and since tests do a lot more db operations than expected we expect to see in actual usage, we disable sqlite's syncing behavior to make db operations run much faster. This syncing behavior is necessary for normal operation as it helps guarantee that data won't become lost or corrupted, but in tests, we don't care about that.
Fixes #21628
ACKs for top commit:
vasild:
ACK 41f891da508114f1fd4df30b4068073ec30abc2a
Tree-SHA512: f36f969a182c622691ae5113573a3250e8d367437e83a1a9d3d2b55dd3a9cdf3c6474169a7bd271007bb9ce47f585aa7a6aeae6eebbaeb02d79409b02f47fd8b
|
|
fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f doc: Remove irrelevant link to GitHub (MarcoFalke)
fa121b628d51bb0e25eb3fbd716881fa55527dc7 blockstorage: [refactor] Use chainman reference where possible (MarcoFalke)
fa0c7d9ad24d3c9515d3f9c136af4071cbd79055 move-only: Move *Disk functions to blockstorage (MarcoFalke)
fa91b2b2b3447a3645e7958c7dc4e1946a69cb9c move-only: Move AbortNode to shutdown (MarcoFalke)
fa413f07a14744e7d7f7746e861aabd9cf938f61 move-only: Move ThreadImport to blockstorage (MarcoFalke)
faf843c07f99f91603e08ea858f972516f1d669a refactor: Move load block thread into ChainstateManager (MarcoFalke)
Pull request description:
This picks up the closed pull request #21030 and is the first step toward fixing #21220.
The basic idea is to move all disk access into a separate module with benefits:
* Breaking down the massive files init.cpp and validation.cpp into logical units
* Creating a standalone-module to reduce the mental complexity
* Pave the way to fix validation related circular dependencies
* Pave the way to mock disk access for testing, especially where it is performance critical (like fuzzing)
ACKs for top commit:
promag:
Code review ACK fadcd3f78e, checked (almost) moved only changes. This is a nice tidy up change and doesn't change behavior. Easily reviewed commit by commit.
jamesob:
ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f ([`jamesob/ackr/21575.1.MarcoFalke.refactor_create_blocksto`](https://github.com/jamesob/bitcoin/tree/ackr/21575.1.MarcoFalke.refactor_create_blocksto))
ryanofsky:
Code review ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f. New organization makes sense, moves extraneous things outside of validation.cpp. PR is also easy to review with helpfully split up moveonly commits.
Tree-SHA512: 917996592b6d8f9998289d8cb2b1b78b23d1fdb3b07216c9caec1380df33baa09dc2c1e706da669d440b497e79c9c62a01ca20dc202df5ad974a75f3ef7a143b
|
|
003929c0d55532038d5bf6fc0ff4a20628710fae refactor: add [[noreturn]] attribute where applicable (fanquake)
Pull request description:
Similar to #10843. We could build with `-Wmissing-noreturn`, however that would also mean modifying something like `--suppress-external-warnings` to suppress warnings for leveldb, which I don't think we want to do. In any case, the functions where this is applicable are only added/removed very rarely.
ACKs for top commit:
vasild:
ACK 003929c0d55532038d5bf6fc0ff4a20628710fae
Tree-SHA512: 33dfa6547d6b84f38a941f24d4c2effe8fde7b93dbc0b27a9309716420e4a879fdbe689d789fa5439d65f5f78292f89fd9dc1b61c97acf69316dfed954086705
|
|
It's not clear why this need it's own exception class, as opposed to just
throwing std::runtime_error().
|
|
|
|
|
|
This is undocumented and unused.
|
|
Rather than 3 different messages that are confusing / leak
implementation details, use a single message, that is similar to other
wallet related messages. i.e:
"Compiled without sqlite support (required for descriptor wallets)".
|
|
|
|
|
|
88d4d5ff2f5c71a9a2f4c78c2b2e2fd00568cfee rpc: add help for enumeratesigners and walletdisplayaddress (Sjors Provoost)
b0db187e5b30a491c9f95685430a82a1e35e921d ci: use --enable-external-signer instead of --with-boost-process (Sjors Provoost)
b54b2e7b1a171203404bd41853372c73f2c64532 Move external signer out of wallet module (Sjors Provoost)
Pull request description:
In addition, this PR enables external signer testing on CI.
This PR moves the ExternalSigner class and RPC methods out of the wallet module.
The `enumeratesigners` RPC can be used without a wallet since #21417. With additional modifications external signers could be used without a wallet in general, e.g. via `signrawtransaction`.
The `signerdisplayaddress` RPC is ranamed to `walletdisplayaddress` because it requires wallet context. A future `displayaddress` RPC call without wallet context could take a descriptor argument.
This commit fixes a `rpc_help.py` failure when configured with `--disable-wallet`.
ACKs for top commit:
ryanofsky:
Code review ACK 88d4d5ff2f5c71a9a2f4c78c2b2e2fd00568cfee
fanquake:
ACK 88d4d5ff2f5c71a9a2f4c78c2b2e2fd00568cfee
Tree-SHA512: 3242a24e22313aed97eee32a520bfcb1c17495ba32a2b8e06a5e151e2611320e2da5ef35b572d84623af0a49a210d2f9377a2531250868d1a0ccf3e144352a97
|