| Age | Commit message (Collapse) | Author |
|---|
|
Moves the VersionBits* functions to be methods of the cache class,
and makes the cache and its lock private to the class.
|
|
Rename BIP9SoftForkPushBack and BuriedSoftForkPushBack to SoftForkPushBack
and have the compiler figure out which one to use based on the deployment
type. Avoids the need to update the file when burying a deployment.
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i -e 's/versionbitscache/g_versionbitscache/g' $(git grep -l versionbitscache)
-END VERIFY SCRIPT-
|
|
Adds support for versionbits deployments to DeploymentEnabled,
DeploymentActiveAfter and DeploymentActiveAt. Also moves versionbitscache
from validation to deploymentstatus.
|
|
Provides DeploymentEnabled, DeploymentActiveAt, and DeploymentActiveAfter
helpers for checking the status of buried deployments. Can be overloaded
so the same syntax works for non-buried deployments, allowing future
soft forks to be changed from signalled to buried deployments without
having to touch the implementation code.
Replaces IsWitnessEnabled and IsScriptWitnessEnabled.
|
|
|
|
fa0d9211ef87a682573aaae932c0c440acbcb8a8 refactor: Remove chainparams arg from CChainState member functions (MarcoFalke)
fa389471251f043ec25e7b01e59b37d3b921ce54 refactor: Remove ::Params() global from inside CChainState member functions (MarcoFalke)
Pull request description:
The `::Params()` global is verbose and confusing. Also it makes tests a bit harder to write because they'd have to mock a global.
Fix all issues by simply using a member variable that points to the right params.
(Can be reviewed with `--word-diff-regex=.`)
ACKs for top commit:
jnewbery:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8
kiminuo:
utACK fa0d9211
theStack:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8 š
Tree-SHA512: 44676b19c9ed471ccb536331d3029bad192d7d50f394fd7b8527ec431452aeec8c4494164b9cf8e16e0123c4463b16be864366c6b599370032c17262625a0356
|
|
before inferring descriptors
8c4129b4540f4f739413ed9a6fbfc78afc252f42 rpc: reset scantxoutset progress on finish (Pavol Rusnak)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/19361 by moving resetting the `g_scan_progress` variable **before** inferring the descriptors
ACKs for top commit:
achow101:
Code review ACK 8c4129b4540f4f739413ed9a6fbfc78afc252f42
Tree-SHA512: 2b81e5c930b9c201a1b9cc742c37e0617582326acd9adbd6b14991d33c05d51b1d5ff6a26f3accab7d274dc8c6a48bc8cbccf0811b60c1f76dd805f9b31482c0
|
|
support for fetching bech32m addresses
754f134a50cc56cdf0baf996d909c992770fcc97 wallet: Add error message to GetReservedDestination (Andrew Chow)
87a0e7a3b7c0ffd545e537bd497cdc3e67d045f6 Disallow bech32m addresses for legacy wallet things (Andrew Chow)
6dbe4d10728f882986ed0d9ed77bc736f051c662 Use BECH32M for tr() desc, WitV1Taproot, and WitUnknown CTxDests (Andrew Chow)
699dfcd8ad9487a4e04c1ffc68211e84e126b3d2 Opportunistically use bech32m change addresses if available (Andrew Chow)
0262536c34567743e527dad46912c9ba493252cd Add OutputType::BECH32M (Andrew Chow)
177c15d2f7cd5406ddbce8217fc023057539b828 Limit LegacyScriptPubKeyMan address types (Andrew Chow)
Pull request description:
Currently bech32m addresses are classfied as bech32. Because bech32m is incompatible with bech32, we need to define a new `OutputType` for it so that it can be handled correctly. This PR adds `OutputType::BECH32M`, updates all of the relevant `OutputType` classifications, and handle requests for bech32m addresses. There is now a `bech32m` address type string that can be used.
* `tr()` descriptors now report their output type as `OutputType::BECH32M`. `WtinessV1Taproot` and `WitnessUnknown` are also classified as `OutputType::BECH32M`.
* Bech32m addresses are completely disabled for legacy wallets. They cannot be imported (explicitly disallowed in `importaddress` and `importmulti`), will not be created when getting all destinations for a pubkey, and will not be added with `addmultisigaddress`. Additional protections have been added to `LegacyScriptPubKeyMan` to disallow attempting to retrieve bech32m addresses.
* Since Taproot multisigs are not implemented yet, `createmultisig` will also disallow the bech32m address type.
* As Taproot is not yet active, `DescriptorScriptPubKeyMan` cannot and will not create a `tr()` descriptor. Protections have been added to make sure this cannot occur.
* The change address type detection algorithm has been updated to return `bech32m` when there is a segwit v1+ output script and the wallet has a bech32m `ScriptPubKeyMan`, falling back to bech32 if one is not available.
ACKs for top commit:
laanwj:
re-review ACK 754f134a50cc56cdf0baf996d909c992770fcc97
Sjors:
re-utACK 754f134: only change is switching to `bech32m` in two `wallet_taproot.py` test cases.
fjahr:
re-ACK 754f134a50cc56cdf0baf996d909c992770fcc97
jonatack:
ACK 754f134a50cc56cdf0baf996d909c992770fcc97
Tree-SHA512: 6ea90867d3631d0d438e2b08ce6ed930f37d01323224661e8e38f183ea5ee2ab65b5891394a3612c7382a1aff907b457616c6725665a10c320174017b998ca9f
|
|
7ad414f4bfa74595ee5726e66f3527045c02a977 doc: add comment about CCoinsViewDBCursor constructor (James O'Beirne)
0f8a5a4dd530549d37c43da52c923ac3b2af1a03 move-only(ish): don't expose CCoinsViewDBCursor (James O'Beirne)
615c1adfb07b9b466173166dc2e53ace540e4b32 refactor: wrap CCoinsViewCursor in unique_ptr (James O'Beirne)
Pull request description:
I tripped over this one for a few hours at the beginning of the week, so I've sort of got a personal vendetta against `CCoinsView::Cursor()` returning a raw pointer.
Specifically in the case of CCoinsViewDB, if a raw cursor is allocated and not freed, a cryptic leveldb assertion failure occurs on CCoinsViewDB destruction (`Assertion 'dummy_versions_.next_ == &dummy_versions_' failed.`).
This is a pretty simple change.
Related to: https://github.com/bitcoin/bitcoin/issues/21766
See also: https://github.com/google/leveldb/issues/142#issuecomment-414418135
ACKs for top commit:
MarcoFalke:
review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 š
jonatack:
re-ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 modulo suggestion
ryanofsky:
Code review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977. Two new commits look good and thanks for clarifying constructor comment
Tree-SHA512: 6471d03e2de674d84b1ea0d31e25f433d52aa1aa4996f7b4aab1bd02b6bc340b15e64cc8ea07bbefefa3b5da35384ca5400cc230434e787c30931b8574c672f9
|
|
We don't want the legacy wallet to ever have bech32m addresses so don't
allow importing them. This includes addmultisigaddress as that is a
legacy wallet only RPC
Additionally, bech32m multisigs are not available yet, so disallow them
in createmultisig.
|
|
Specifically with CCoinsViewDB, if a raw cursor is allocated and
not freed, a cryptic leveldb assertion failure occurs on
CCoinsViewDB destruction.
See: https://github.com/google/leveldb/issues/142#issuecomment-414418135
|
|
Passing this is confusing and redundant with the m_params member.
|
|
For non-Taproot signatures, this is interpreted as SIGHASH_ALL.
|
|
|
|
-BEGIN VERIFY SCRIPT-
find_regex='((assert|CHECK_NONFATAL)\(std::addressof|TODO: REVIEW-ONLY)' \
&& git grep -l -E "$find_regex" -- . \
| xargs sed -i -E "/${find_regex}/d"
-END VERIFY SCRIPT-
|
|
ee862d6efb4c3c01e55f0d5d7a82cce75323cf40 MOVEONLY: context-free package policies (glozow)
5cac95cd15da04b83afa1d31a43be9f5b30a1827 disallow_mempool_conflicts -> allow_bip125_replacement and check earlier (glozow)
e8ecc621be6afd3252c0f8147e42c3b4918f7f46 [refactor] comment/naming improvements (glozow)
7d91442461776e2ef240d7885f768b624de341a7 [rpc] reserve space in txns (glozow)
6c5f19d9c4d267c54f4dbc4f9d65370ff1e0625b [package] static_assert max package size >= max tx size (glozow)
Pull request description:
various followups from #20833
ACKs for top commit:
jnewbery:
utACK ee862d6efb4c3c01e55f0d5d7a82cce75323cf40
ariard:
Code Review ACK ee862d6
Tree-SHA512: 96ecb41f7bbced84d4253070f5274b7267607bfe4033e2bb0d2f55ec778cc41e811130b6321131e0418b5835894e510a4be8a0f822bc9d68d9224418359ac837
|
|
|
|
2667366aaa69447a9de4d819669d254a5ebd4d4b tests: check derivation of P2TR (Pieter Wuille)
7cedafc5412857404e9a6c3450b100cb8ee4081a Add tr() descriptor (derivation only, no signing) (Pieter Wuille)
90fcac365e1616779b40a69736428435df75fdf2 Add TaprootBuilder class (Pieter Wuille)
5f6cc8daa83700d1c949d968a5cf0d935be337b7 Add XOnlyPubKey::CreateTapTweak (Pieter Wuille)
2fbfb1becb3c0c109cd7c30b245b51da22039932 Make consensus checking of tweaks in pubkey.* Taproot-specific (Pieter Wuille)
a4bf84039c00b196b87f969acf6369d72c56ab46 Separate WitnessV1Taproot variant in CTxDestination (Pieter Wuille)
41839bdb89b3777ece2318877b9c7921ecca2472 Avoid dependence on CTxDestination index order (Pieter Wuille)
31df02a07091dbd5e0b315c8e5695e808f3a5505 Change Solver() output for WITNESS_V1_TAPROOT (Pieter Wuille)
4b1cc08f9f94a1e6e1ecba6b97f99b73fb513872 Make XOnlyPubKey act like byte container (Pieter Wuille)
Pull request description:
This is a subset of #21365, to aide review.
This adds support `tr(KEY)` or `tr(KEY,SCRIPT)` or `tr(KEY,{{S1,{{S2,S3},...}},...})` descriptors, describing Taproot outputs with specified internal key, and optionally any number of scripts, in nested groups of 2 inside `{`/`}` if there are more than one. While it permits importing `tr(KEY)`, anything beyond that is just laying foundations for more features later.
Missing:
* Signing support (see #21365)
* Support for more interesting scripts inside the tree (only `pk(KEY)` is supported for now). In particular, a multisig policy based on the new `OP_CHECKSIGADD` opcode would be very useful.
* Inferring `tr()` descriptors from outputs (given sufficient information).
* `getaddressinfo` support.
* MuSig support. Standardizing that is still an ongoing effort, and is generally kind of useless without corresponding PSBT support.
* Convenient ways of constructing descriptors without spendable internal key (especially ones that arent't trivially recognizable as such).
ACKs for top commit:
Sjors:
utACK 2667366 (based on https://github.com/bitcoin/bitcoin/pull/21365#issuecomment-846945215 review, plus the new functional test)
achow101:
Code Review ACK 2667366aaa69447a9de4d819669d254a5ebd4d4b
lsilva01:
Tested ACK https://github.com/bitcoin/bitcoin/pull/22051/commits/2667366aaa69447a9de4d819669d254a5ebd4d4b
meshcollider:
utACK 2667366aaa69447a9de4d819669d254a5ebd4d4b
Tree-SHA512: 61046fef22c561228338cb178422f0b782ef6587ec8208d3ce2bd07afcff29a664b54b35c6b01226eb70b6540b43f6dd245043d09aa6cb6db1381b6042667e75
|
|
|
|
|
|
auxiliary modules
7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a index: refactor-only: Reuse CChain ref (Carl Dong)
db33cde80fff749c6adff9e91fca5f27f4bb6278 index: Add chainstate member to BaseIndex (Carl Dong)
f4a47a1febfa35ab077f2a841fe31a8cd9618250 bench: Use existing chainman in AssembleBlock (Carl Dong)
91226eb91769aad5a63bc671595e1353a2b2247a bench: Use existing NodeContext in DuplicateInputs (Carl Dong)
e6b4aa6eb53dc555ecab2922af35e7a2572faf4f miner: Pass in chainman to RegenerateCommitments (Carl Dong)
9ecade14252ad1972f668d2d2e4ef44fdfcb944a rest: Add GetChainman function and use it (Carl Dong)
fc1c282845f6b8436d1ea4c68eb3511034c29bea rpc/blockchain: Use existing blockman in gettxoutsetinfo (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
The first 2 commits are fixups addressing review for the last bundle: #21391
NEW note:
1. I have opened #21766 which keeps track of potential improvements where the flaws already existed before the de-globalization work, please post on that issue about these improvements, thanks!
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:
jarolrod:
ACK 7a799c9
ariard:
Code Review ACK 7a799c9
fjahr:
re-ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a
MarcoFalke:
review ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a š
ryanofsky:
Code review ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a. Basically no change since last review except fixed rebase conflicts and a new comment about REST Ensure()
jamesob:
conditional ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a ([`jamesob/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai`](https://github.com/jamesob/bitcoin/tree/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai))
Tree-SHA512: 531c00ddcb318817457db2812d9a9d930bc664e58e6f7f1c746350732b031dd624270bfa6b9f49d8056aeb6321d973f0e38e4ff914acd6768edd8602c017d10e
|
|
testmempoolaccept
13650fe2e527bf0cf5d977bf5f3f1563b853ecdc [policy] detect unsorted packages (glozow)
9ef643e21b44f99f4bce54077788d0ad4d81f7cd [doc] add release note for package testmempoolaccept (glozow)
c4259f4b7ee23ef6e0ec82c5d5b9dfa9cadd5bed [test] functional test for packages in RPCs (glozow)
9ede34a6f20378e86c5289ebd20dd394a5915123 [rpc] allow multiple txns in testmempoolaccept (glozow)
ae8e6df709ff3d52b8e9918e09cacb64f83ae379 [policy] limit package sizes (glozow)
c9e1a26d1f17c8b98632b7796ffa8f8788b5a83c [fuzz] add ProcessNewPackage call in tx_pool fuzzer (glozow)
363e3d916cc036488783bb4bdcfdd3665aecf711 [test] unit tests for ProcessNewPackage (glozow)
cd9a11ac96c01e200d0086b2f011f4a614f5a705 [test] make submit optional in CreateValidMempoolTransaction (glozow)
2ef187941db439c5b3e529f08b6ab153ff061fc5 [validation] package validation for test accepts (glozow)
578148ded62828a9820398165c41670f4dbb523d [validation] explicit Success/Failure ctors for MempoolAcceptResult (glozow)
b88d77aec5e7bef5305a668d15031351c0548b4d [policy] Define packages (glozow)
249f43f3cc52b0ffdf2c47aad95ba9d195f6a45e [refactor] add option to disable RBF (glozow)
897e348f5987eadd8559981a973c045c471b3ad8 [coins/mempool] extend CCoinsViewMemPool to track temporary coins (glozow)
42cf8b25df07c45562b7210e0e15c3fd5edb2c11 [validation] make CheckSequenceLocks context-free (glozow)
Pull request description:
This PR enables validation dry-runs of packages through the `testmempoolaccept` RPC. The expectation is that the results returned from `testmempoolaccept` are what you'd get from test-then-submitting each transaction individually, in that order (this means the package is expected to be sorted in topological order, for now at least). The validation is also atomic: in the case of failure, it immediately halts and may return "unfinished" `MempoolAcceptResult`s for transactions that weren't fully validated. The API for 1 transaction stays the same.
**Motivation:**
- This allows you to test validity for transaction chains (e.g. with multiple spending paths and where you don't want to broadcast yet); closes #18480.
- It's also a first step towards package validation in a minimally invasive way.
- The RPC commit happens to close #21074 by clarifying the "allowed" key.
There are a few added restrictions on the packages, mostly to simplify the logic for areas that aren't critical to main package use cases:
- No package can have conflicts, i.e. none of them can spend the same inputs, even if it would be a valid BIP125 replacement.
- The package cannot conflict with the mempool, i.e. RBF is disabled.
- The total count of the package cannot exceed 25 (the default descendant count limit), and total size cannot exceed 101KvB (the default descendant size limit).
If you're looking for review comments and github isn't loading them, I have a gist compiling some topics of discussion [here](https://gist.github.com/glozow/c3acaf161c95bba491fce31585b2aaf7)
ACKs for top commit:
laanwj:
Code review re-ACK 13650fe2e527bf0cf5d977bf5f3f1563b853ecdc
jnewbery:
Code review ACK 13650fe2e527bf0cf5d977bf5f3f1563b853ecdc
ariard:
ACK 13650fe
Tree-SHA512: 8c5cbfa91a6c714e1c8710bb281d5ff1c5af36741872a7c5df6b24874d6272b4a09f816cb8a4c7de33ef8e1c2a2c252c0df5105b7802f70bc6ff821ed7cc1a2f
|
|
Pass in chainman instead of prev_block so that we can enforce the
block.hashPrevBlock refers to prev_block invariant in the function
itself.
We should probably rethink BlockAssembler's API and somehow include
commitment regeneration functionality in there. Something like a variant
of CreateNewBlock that takes in a std::vector<TxRef> and return a CBlock
instead of CBlockTemplate. That could avoid reaching for
LookupBlockIndex at all.
|
|
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
|
|
|
|
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.
|
|
|
|
-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-
|
|
getnodeaddresses by network
ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d doc: release note for getnodeaddresses by network (Jon Atack)
3f89c0e9902338ad8a507a938dceeeb3191eece6 test: improve getnodeaddresses coverage, test by network (Jon Atack)
6c98c099918bd20e2d3aa123643d6e3594e080e4 rpc: enable filtering getnodeaddresses by network (Jon Atack)
80ba294854e5025bcada58f1403858e6ea1d4380 p2p: allow CConnman::GetAddresses() by network, add doxygen (Jon Atack)
a49f3ddbbabfb971a537f0a6c7affb24e20ff192 p2p: allow CAddrMan::GetAddr() by network, add doxygen (Jon Atack)
c38981e748f438d972ba12ba998c8a8a597e01c1 p2p: pull time call out of loop in CAddrMan::GetAddr_() (JoĆ£o Barbosa)
d35ddca91ebbcf8d8b790c3b9f8cf218fafb7a53 p2p: enable CAddrMan::GetAddr_() by network, add doxygen (Jon Atack)
Pull request description:
This patch allows passing a network argument to CAddrMan::GetAddr(), CConnman::GetAddresses(), and rpc getnodeaddresses to return only addresses of that network.
It also contains a performance optimisation by promag.
ACKs for top commit:
laanwj:
Code review and lightly tested ACK ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d
vasild:
ACK ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d
Tree-SHA512: 40e700d97091248429c73cbc0639a1f03ab7288e636a7b9026ad253e9708253c6b2ec98e7d9fb2d56136c0f762313dd648915ac98d723ee330d713813a43f99d
|
|
6e2eb0d63b42288c11a65d585d487108643888d0 rpc/wallet: use OMITTED_NAMED_ARG instead of Default(VNULL) (Karl-Johan Alm)
4983f4cba44c4ffaa4972fdede7cf6fcf8caec00 rpc/createwallet: omitted named arguments (Karl-Johan Alm)
dc4db23b30b4bc7884bb28630b2b24edd81c1799 rpc: address:amount dictionaries are OBJ_USER_KEYS (Karl-Johan Alm)
c8cf0a3d513b8c892f1ae16b8c0cda184064a07b rpc/getpeerinfo: bytesrecv_per_msg is a dynamic dictionary (Karl-Johan Alm)
eb4fb7e507b583bd4ae8d1e3747f41616c782ded rpc/gettxoutsetinfo: hash_or_height is a named argument (Karl-Johan Alm)
Pull request description:
This is a follow-up to #21897, and I believe covers the remaining cases, at least that I could find.
Edited to remove unrelated information about a side project.
ACKs for top commit:
laanwj:
Documentation diff ACK 6e2eb0d63b42288c11a65d585d487108643888d0
promag:
Code review ACK 6e2eb0d63b42288c11a65d585d487108643888d0.
Tree-SHA512: d26f6e074e13d64bbca2a114a0adc7f905d47d238c4e9bc49f70ca0b775afbebf9879fc3794ab29dc316a6dbd00ba8cbeb01197e236ee4ab2e9854db25f23f04
|
|
Was missed in last bundle
|
|
|
|
|
|
|
|
|
|
It is not a dictionary with the single key 'msg'.
|
|
|
|
fae196147bae11202c0d54543dc12ba5d92ab0cc doc: Clarify that feerates are per virtual size (MarcoFalke)
fa83e95ac6f318caa38016a08fa4e402c3b05833 scripted-diff: Clarify that feerates are per virtual size (MarcoFalke)
Pull request description:
By implementing segwit, it is already clear that all feerates in Bitcoin Core are denoted in (amount/virtual size). Though, there is inconsistency, as some places use kvB, some use kB. Thus, replace all with "kvB".
See also commit 6da3afbaee5809ebf6d88efaa3958c505c2d71c7, which did the replacement for wallet RPCs.
ACKs for top commit:
ryanofsky:
Code review ACK fae196147bae11202c0d54543dc12ba5d92ab0cc. Checked instances where units were being added in the second commit and they all looked right.
Tree-SHA512: ab70d13cde7d55c1ac931bddc2b45aa218fc75ef46cb6ea9e5a30b1d4dbf27889c2b6357299a6c5427912443a46ec3592a4809dae335e03162bd2120a0f7f8ad
|
|
|
|
(mantissa of 3)
847288df07b45ca535c849e518b22818ab492896 test: fee rate values that cannot be represented as sat/vB (Jon Atack)
06a90fa0381c790f7bde2ab9bf47d2b22acef4a5 rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3 (Jon Atack)
0742c7840f03505597fd2de87db97f12597ef667 rpc: enable passing decimals to AmountFromValue, add doxygen (Jon Atack)
8ce3ef57a3e9ad13c0aaa4648e8584241d53592d test: ParseFixedPoint with 3 decimals for sat/vB fee rates (Jon Atack)
b5033275979a2a495b02b25f70cadbdcc8b6eb6a test: type error and out of range fee rates where missing (Jon Atack)
c5fd4344f7fcc257062a610c8ff26ffcc9b53953 test: explicit fee rates with invalid amounts (Jon Atack)
ea6f76b66ecc52360719053489e0ec9f9a673eab test: improve zero-value explicit fee rate coverage (Jon Atack)
Pull request description:
- Improve/close gaps in existing test coverage before making the change
- Enable passing `decimals` to `ParseFixedPoint()` when calling `AmountFromValue()`
- Limit explicit fee rates in sat/vB passed in by users to 3 decimals, and raise otherwise
- Add regression test coverage
Closes #20534.
ACKs for top commit:
MarcoFalke:
review ACK 847288df07b45ca535c849e518b22818ab492896 š·
Tree-SHA512: c539d07ae9b21c0d6c8ea460beb9c8dad5559445518aace560abc3c05c588907bae189b6fd7602b3b397de4a42356136c3ec6f960d3dcf2d5d16377aef4ab5a2
|
|
7031721f2cc3eef30c46ff50c52328e9ba8090e0 rpc/listaddressgroupings: redefine inner-most array as ARR_FIXED (Karl-Johan Alm)
8500f7bf54d3e27fd2fa7fda15ad833f5688c331 rpc/createrawtransaction: redefine addresses as OBJ_USER_KEYS (Karl-Johan Alm)
d9e2183c50f50465b9f173171fee240949bf8bd2 rpc: include OBJ_USER_KEY in RPCArg constructor checks (Karl-Johan Alm)
Pull request description:
This PR adjusts the two issues I encountered while developing a tool that converts RPCHelpMan objects into bindings for other language(s).
The first is in createrawtransaction, where the address part, e.g. bc1qabc in
> createrawtransaction '[]' '[{"bc1qabc": 1.0}]'
is declared as a `Type::OBJ`, when in reality it should be a `Type::OBJ_USER_KEYS`, defined as such:
https://github.com/bitcoin/bitcoin/blob/5925f1e652768a9502831b9ccf78d16cf3c37d29/src/rpc/util.h#L126
(coincidentally, this is the first and only (afaict) usage of this `RPCArg::Type`).
The second is in the `listaddressgroupings` RPC, which returns an array of arrays of arrays, where the innermost one is a tuple-thingie with an optional 3rd item; this is an `ARR_FIXED`, not an `ARR`.
ACKs for top commit:
MarcoFalke:
ACK 7031721f2cc3eef30c46ff50c52328e9ba8090e0 š
Tree-SHA512: 769377416c6226d1738a956fb685498e009f9e7eb2d45bc679b81c5364b9520fdbcb49392c937ab45598aa0d33589e8e6a59ccc101cf8d8e7dfdafd58d4eefd0
|
|
The OBJ type is for actual objects with defined keys; OBJ_USER_KEYS is for objects with user-defined keys (such as the bitcoin address(es) in the createrawtransaction output object.
|
|
|
|
|
|
-addnode/-maxconnections config options
b4fcbcfb49461b96bc72fb64d6152de7c5ce00de doc: update -maxconnections config option help (Jon Atack)
79685a8992ad302833b506cc6d03aab1cc127de0 doc: update -addnode config option help (Jon Atack)
2896c6c4cc6d382d8369c037e274c08dd8e32c69 doc: update addnode rpc help (Jon Atack)
Pull request description:
Since #9319 proposed by Gregory Maxwell and released in v0.14, peers manually added through the `-addnode` config option or using the `addnode` RPC have their own separate limit of 8 connections that does not compete with other inbound or outbound connection usage and is not subject to the limitation imposed by the `-maxconnections` option.
This PR updates the `-addnode` and `-maxconnections` config options and the `addnode` RPC help docs with this information.
`-addnode` config option help
```
$ bitcoind -h | grep -A5 addnode=
-addnode=<ip>
Add a node to connect to and attempt to keep the connection open (see
the addnode RPC help for more info). This option can be specified
multiple times to add multiple nodes; connections are limited to
8 at a time and are counted separately from the -maxconnections
limit.
$ bitcoind -h | grep -A3 maxconnections=
-maxconnections=<n>
Maintain at most <n> connections to peers (default: 125). This limit
does not apply to connections manually added via -addnode or the
addnode RPC, which have a separate limit of 8.
```
`addnode` rpc help
```
$ bitcoin-cli help addnode
addnode "node" "command"
Attempts to add or remove a node from the addnode list.
Or try a connection to a node once.
Nodes added using addnode (or -connect) are protected from DoS disconnection and are not required to be
full nodes/support SegWit as other outbound peers are (though such peers will not be synced from).
Addnode connections are limited to 8 at a time and are counted separately from the -maxconnections limit.
```
ACKs for top commit:
prayank23:
ACK https://github.com/bitcoin/bitcoin/commit/b4fcbcfb49461b96bc72fb64d6152de7c5ce00de
jarolrod:
ACK b4fcbcfb49461b96bc72fb64d6152de7c5ce00de
Tree-SHA512: b6d69baa6cbf6d53f91bac5b39b549d49db6c95f92ea1bdd3588a6432794a25ac2c8b3c89e2c72bb9097e61f2717c8b5ecc404745d5992b88e523db03200898f
|
|
context
ebd4be43cc945e643f91d3a91007b5a35bbbd5a1 doc: add release notes for 20867 (Antoine Poinsot)
5aa50ab9cc7994b16cf13e4c73af80f0098f1bea rpc/util: multisig: only check redeemScript size is <= 520 for P2SH (Antoine Poinsot)
063df9e89730fd2c92646577e2fab894e1692130 test/functional: standardness sanity checks for P2(W)SH multisig (Antoine Poinsot)
ae0429d3af6de48f6191f144dff4ad4ab672dcd6 script: allow up to 20 keys in wsh() descriptors (Antoine Poinsot)
9fc68faf35c700ae955af194dd7f8c1aee85a05b script: match multisigs with up to MAX_PUBKEYS_PER_MULTISIG keys (Antoine Poinsot)
Pull request description:
As described in https://github.com/bitcoin/bitcoin/issues/20620 multisigs are currently limited to 16 keys in descriptors and RPC helpers, even for P2WSH and P2SH-P2WSH.
This adds support for multisig with up to 20 keys (which are already standard) for Segwit v0 context for descriptors (`wsh()`, `sh(wsh())`) and RPC helpers.
Fixes https://github.com/bitcoin/bitcoin/issues/20620
ACKs for top commit:
meshcollider:
re-utACK ebd4be43cc945e643f91d3a91007b5a35bbbd5a1
instagibbs:
re-ACK https://github.com/bitcoin/bitcoin/pull/20867/commits/ebd4be43cc945e643f91d3a91007b5a35bbbd5a1
Tree-SHA512: 36141f10a8288010d17d5c4fe8d24878bcd4533b88a8aba3a44fa8f74ceb3182d70fee01427e0ab7f53ce7fab46c88c1cd3ac3b18ab8a10bd4a6b8b74ed79e46
|
|
|
|
|
