d0a3feea73 Change docs for walletcreatefundedpsbt RPC method (Ivan Vershigora)
Pull request description:
`sequence` field in the list of inputs currently marked as "required". Actually it can be omitted and it's value depends on `locktime` and `options.replaceable` fields. Just the same as in `createpsbt` call.
ACKs for top commit:
achow101:
ACK d0a3feea73
Tree-SHA512: 3f429a2c2eea283a47fb5002a99f7e2a5ed6f67df9fd895c1ab938256c48a6497ed6ac2673d8fe8968dfb67b939f4a84570899d9faf52f3abd6ec90c0703d1bd
script/standard.cpp:278:22: error: default initialization of an object of const type 'const (anonymous namespace)::CScriptVisitor' without a user-provided default constructor
const CScriptVisitor g_script_visitor;
^
{}
1 error generated.
951bca61d7 tests: feature_backwards_compatibility.py test 0.16 up/downgrade (Andrew Chow)
3a03a11e8c Skip hdKeypath of 'm' (Andrew Chow)
Pull request description:
Previously the seed was stored with keypath 'm' so we need to skip this as well when determining inactive seeds.
Fixes #19051
ACKs for top commit:
Sjors:
ACK 951bca61d7
instagibbs:
re-utACK 951bca61d7
ryanofsky:
Code review ACK 951bca61d7. No significant changes since last review, just updated comment and some test tweaks
Tree-SHA512: 930f77e7097c9cf4f1012e540bd2b1a72fd279262517f10c1531b2ad48c632ef95e0dd4edea81bcc3b3db306479d34e5e79e5d6c4ed31dfa4b77a4231436436e
A templated BaseHash does not allow for automatic conversion, thus
conversions much be explicitly allowed / whitelisted, which will
reduce the risk of unintended conversions.
These types are equivalent, in data etc, so they need only their
data cast across.
Note a function is used rather than a casting
operator as CKeyID is defined at a lower level than script/standard
b83cc0fc94 Fix link error with --enable-debug (Hennadii Stepanov)
Pull request description:
Fixes a link error on master (39bd9ddb87):
```
$ ./configure --enable-debug
$ make
...
bitcoin_wallet-bitcoin-wallet.o:(.data.rel.ro+0x0): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet_tool.a(libbitcoin_wallet_tool_a-wallettool.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-salvage.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-walletdb.o):(.data.rel.ro+0x8): undefined reference to `InitError(bilingual_str const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o):(.data.rel.ro+0x8): more undefined references to `InitError(bilingual_str const&)' follow
collect2: error: ld returned 1 exit status
```
See:
- https://github.com/bitcoin/bitcoin/pull/19295#issuecomment-645471771
- https://github.com/bitcoin/bitcoin/pull/19295#issuecomment-645487182
ACKs for top commit:
achow101:
Re-ACK b83cc0fc94
Tree-SHA512: f563d978b6725284049449bb0b3a184d356f32e9b63bcadb0ba71352d3d521af3dbb4a7b4fc0a5a620ed99c357e59f62249c10d0defc0cbe7775f2c06791dabe
3351c91ed4 refactor: Make CScriptVisitor stateless (João Barbosa)
Pull request description:
`CScriptVisitor` was added in 1025440184 (#1357) and the visitor return type was never used. Now `CScriptVisitor` is stateless and `CScript` is the return type.
ACKs for top commit:
MarcoFalke:
ACK 3351c91ed4🏤
sipa:
utACK 3351c91ed4
Tree-SHA512: d158ad2ebe8ea4dc8cc090b943dd66fa5421a84f9443e16ab2d661df38e1a85de16ff13cbaa56924489d8d43cba25fa3cd8b6904bbbcbf356b886ffe8ffba19a
51e9393c1f refactor: s/command/msg_type/ in CNetMsgMaker and CSerializedNetMsg (Sebastian Falbesoner)
Pull request description:
Follow-up PR for #18533 -- another small step towards getting rid of the confusing "command" terminology. Also see PR #18610 which tackled the functional tests.
ACKs for top commit:
MarcoFalke:
ACK 51e9393c1f
Tree-SHA512: bb6f05a7be6823d5c4eab1d05b31fee944e700946827ad9425d59a3957fd879776c88c606319cbe9832d9451b275baedf913b71429ea3e01e4e82bf2d419e819
Add a "PSBT Operations" dialog, reached from the "Load PSBT..." menu
item, giving options to sign or broadcast the loaded PSBT as
appropriate, as well as copying the result to the clipboard or saving
it to a file.
fa1904e5f0 net: Remove dead logging code (MarcoFalke)
fac12ebf4f net: Avoid redundant and confusing FAILED log (MarcoFalke)
Pull request description:
Remove a redundant and confusing "FAILED" log message and gets rid of the unused return type in `ProcessMessage`
ACKs for top commit:
jnewbery:
utACK fa1904e5f0
gzhao408:
utACK fa1904e5f0
troygiorshev:
ACK fa1904e5f0
naumenkogs:
utACK fa1904e
Tree-SHA512: bfa553d5efa022727ed17877fb7c08c14849d804fe6d6a7ce172d513857beba35de41ea40b27ff1aedf68b81e2cda7b2a948ac985fcaaf1b6cfb96cce4837c90
f52d403b81 [net] split PushInventory() (John Newbery)
Pull request description:
PushInventory() is currently called with a CInv object, which can be a
MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine
whether to add the hash to setInventoryTxToSend or
vInventoryBlockToSend.
Since the caller always knows what type of inventory they're pushing,
the CInv is wastefully constructed and thrown away, and tx/block relay
is being split out, we split the function into PushTxInventory() and
PushBlockInventory().
ACKs for top commit:
amitiuttarwar:
utACK f52d403b81. nice cleanup, this has bothered me :)
naumenkogs:
utACK f52d403
sipa:
utACK f52d403b81
Tree-SHA512: 331495199a3b1a2620e6a62beb336e494291b725d8fd64bb44726c02e80807f3974ff4f329bb0f059088e65cd7d41eff276c1065806d2dd6e72c5a9f368e82cd
In FillPSBT, optionally report the number of inputs we successfully
signed, as an out parameter. If "sign" is false, instead report the
number of inputs for which GetSigningProvider does not return nullptr.
(This is a potentially overbroad estimate of inputs we could sign.)
83fd3a6d73 init: use std::thread for ThreadImport() (fanquake)
Pull request description:
[Mentioned](https://github.com/bitcoin/bitcoin/pull/19142#issuecomment-638090759) in #19142, which removed the `boost::interruption_point()`
in `ThreadImport()`.
ACKs for top commit:
hebasto:
ACK 83fd3a6d73, I have reviewed the code and it looks OK, I agree it can be merged.
donaloconnor:
ACK 83fd3a6
laanwj:
Code review ACK 83fd3a6d73
MarcoFalke:
ACK 83fd3a6d73
Tree-SHA512: 0644947d669feb61eed3a944012dad1bd3dd75cf994aa2630013043c213a335b162b63e20aa37e0997740d8e3a3ec367b660b5196007a09e13f0ac455b36c821
da7a83c5ee Remove WalletDatabase::Create, CreateMock, and CreateDummy (Andrew Chow)
d6045d0ac6 scripted-diff: Replace WalletDatabase::Create* with CreateWalletDatabase (Andrew Chow)
45c08f8a7b Add Create*WalletDatabase functions (Andrew Chow)
Pull request description:
Instead of having `Create`, `CreateMock`, and `CreateDummy` being static functions in `BerkeleyDatabase`, move these to standalone functions in `walletdb.cpp`. This prepares us for having different `WalletDatabase` classes.
Part of #18971. This was originally one commit but has been split into 3 to make it (hopefully) easier to review.
ACKs for top commit:
MarcoFalke:
ACK da7a83c5ee🎂
ryanofsky:
Code review ACK da7a83c5ee. Easy review, nice scripted-diff
Tree-SHA512: 1feb7cb3889168c555154bf3701a49095fd6b8cab911d44b7f7efbf6fcee2280ccb3d4afec8a83755b39a592ecd13b90a318faa655c321f87bdabdf1e2312327
PushInventory() is currently called with a CInv object, which can be a
MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine
whether to add the hash to setInventoryTxToSend or
vInventoryBlockToSend.
Since the caller always knows what type of inventory they're pushing,
the CInv is wastefully constructed and thrown away, and tx/block relay
is being split out, we split the function into PushTxInventory() and
PushBlockInventory().
a389ed52e8 walletdb: refactor Read, Write, Erase, and Exists into non-template func (Andrew Chow)
Pull request description:
In order to override these later, the specific details of how the Read, Write, Erase, and Exists functions interact with the actual database file need to go into functions that are not templated.
The functions `ReadKey`, `WriteKey`, `EraseKey`, and `HasKey` are introduced to handle the actual interaction with the database.
This is mostly a moveonly.
Based on #19290
ACKs for top commit:
ryanofsky:
Code review ACK a389ed52e8. No changes since last review, just non-conflicting rebase
Sjors:
utACK a389ed52e8
MarcoFalke:
ACK a389ed52e8🔳
Tree-SHA512: 73bd2fe9ddc4a132d4db6b97e77f5d5f8aa68b8cb25192384f3bacd826365947763a9eee73672331d34578e3f5ade85ee6aa550ff4d89eb62e482250dd5973e4
26acc8dd9b Add sanity check asserts to span when -DDEBUG (Pieter Wuille)
2676aeadfa Simplify usage of Span in several places (Pieter Wuille)
ab303a16d1 Add Span constructors for arrays and vectors (Pieter Wuille)
bb3d38fc06 Make pointer-based Span construction safer (Pieter Wuille)
1f790a1147 Make Span size type unsigned (Pieter Wuille)
Pull request description:
This improves our Span class by making it closer to the C++20 `std::span` one:
* ~~Support conversion between compatible Spans (e.g. `Span<char>` to `Span<const char>`).~~ (done in #18591)
* Make the size type `std::size_t` rather than `std::ptrdiff_t` (the C++20 one underwent the same change).
* Support construction of Spans directly from arrays, `std::string`s, `std::array`s, `std::vector`s, `prevector`s, ... (for all but arrays, this only works for const containers to prevent surprises).
And then make use of those improvements in various call sites.
I realize the template magic used looks scary, but it's only needed to make overload resultion make the right choices. Note that the operations done on values are all extremely simple: no casts, explicit conversions, or warning-silencing constructions. That should hopefully make it simpler to review.
ACKs for top commit:
laanwj:
Code review ACK 26acc8dd9b
promag:
Code review ACK 26acc8dd9b.
Tree-SHA512: 5a5bd346a140edf782b5b3b3f04d9160c7b9e9def35159814a07780ab1dd352545b88d3cc491e0f80d161f829c49ebfb952fddc9180f1a56f1257aa51f38788a
In order to override these later, the specific details of how the Read,
Write, Erase, and Exists functions interact with the actual database
file need to go into functions that are not templated.
fa02b47313 refactor: Use AbortError in FatalError (MarcoFalke)
Pull request description:
`FatalError` has been copied from `AbortNode`, so the two should use the same style to avoid confusion.
Follow-up to #18927
ACKs for top commit:
hebasto:
ACK fa02b47313, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 2cf6d18a6ffb5c2e5cf54f0a072a7cef6dc7e924152b2fee44e6ff2c6c53bad962afd364eda30d8a73883d656429ea68391090e6a27057e69eaefd7c4dad0a33
f8213c05f0 Add means to handle negative capabilities in thread safety annotations (Hennadii Stepanov)
Pull request description:
This commit is separated from #19238, and it adds support of [Negative Capabilities](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative) in the Clang Thread Safety Analysis attributes.
> Negative requirements are an alternative `EXCLUDES` [`LOCKS_EXCLUDED`] that provide a stronger safety guarantee. A negative requirement uses the `REQUIRES` [`EXCLUSIVE_LOCKS_REQUIRED`] attribute, in conjunction with the ! operator, to indicate that a capability should not be held.
Examples of usage:
- #19238 (for a class)
- https://github.com/hebasto/bitcoin/tree/200610-addrman-tsn (for the whole code base)
ACKs for top commit:
MarcoFalke:
Approach ACK f8213c05f0
vasild:
ACK f8213c05
Tree-SHA512: 86d992826b87579661bd228712ae5ee6acca6f70b885ef7e96458974eac184e4874a525c669607ba6b6c861aa4806409a8792d100e6914c858bcab43d31cfb1b
61c16339da walletdb: Move BDB specific things into bdb.{cpp/h} (Andrew Chow)
8f033642a8 walletdb: moveonly: Move BerkeleyBatch Cursor and Txn funcs to cpp (Andrew Chow)
25a655794a walletdb: move IsWalletLoaded to walletdb.cpp (Andrew Chow)
f6fc5f3849 walletdb: Add IsBDBWalletLoaded to look for BDB wallets specifically (Andrew Chow)
c3538f435a walletdb: Make SpliWalletFilePath non-static (Andrew Chow)
Pull request description:
Moves the BDB specific classes from db.{cpp/h} to bdb.{cpp/h}.
To do this, `SplitWalletFilePath` is first made non-static. Then `IsWalletLoaded` functionality is moved to `IsBDBWalletLoaded` which is called by `IsWalletLoaded`. Then the bulk of db.{cpp/h} is moved to a new file bdb.{cpp/h}.
While doing some moveonly stuff, an additional commit moves the `*Cursor` and `Txn*` implementations out of the header file and into the cpp file.
Part of #18971
ACKs for top commit:
laanwj:
Code review ACK 61c16339da
promag:
Code review ACK 61c16339da.
meshcollider:
utACK 61c16339da
Tree-SHA512: cb676cd34c9cd3c838a4fef230d84711efe4cf0d2eefa64ebfd7f787ddc6f7379db0b29454874ddc46ca7ffee0f18f6f3fb96a85513cd10164048948fd03a80c
