| Age | Commit message (Collapse) | Author |
|---|
|
existing fuzzing harness
7e9c7113afbed96cef80c327cc93e82000d6bb69 compressor: Make the domain of CompressAmount(...) explicit (practicalswift)
4a7fd7a7124f84e010b01d0769ef0572bf031ee8 tests: Add amount compression/decompression fuzzing to existing fuzzing harness: test compression round-trip (practicalswift)
Pull request description:
Small fuzzing improvement:
Add amount compression/decompression fuzzing to existing fuzzing harness: test compression round-trip (`DecompressAmount(CompressAmount(…))`).
Make the domain of `CompressAmount(…)` explicit.
Amount compression primer:
```
Compact serialization for amounts
Special serializer/deserializer for amount values. It is optimized for
values which have few non-zero digits in decimal representation. Most
amounts currently in the txout set take only 1 or 2 bytes to
represent.
```
**How to test this PR**
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/integer
…
```
Top commit has no ACKs.
Tree-SHA512: 0f7c05b97012ccd5cd05a96c209e6b4d7d2fa73138bac9615cf531baa3f614f9003e29a198015bcc083af9f5bdc752bb52615b82c5df3c519b1a064bd4fc6664
|
|
|
|
fuzzing strprintf(...)
470e2ac602ed2d6e62e5c80f27cd0a60c7cf6bce tests: Avoid hitting some known minor tinyformat issues when fuzzing strprintf(...) (practicalswift)
Pull request description:
Avoid hitting some known minor tinyformat issues when fuzzing `strprintf(...)`. These can be removed when the issues have been resolved upstreams :)
Note to reviewers: The `%c` and `%*` issues are also present for `%<some junk>c` and `%<some junk>*`. That is why simply matching on `"%c"` or `"%*"` is not enough. Note that the intentionally trivial skipping logic overshoots somewhat (`c[…]%` is filtered in addition to `%[…]c`).
Top commit has no ACKs.
Tree-SHA512: 2b002981e8b3f2ee021c3013f1260654ac7e158699313849c9e9660462bb8cd521544935799bb8daa74925959dc04d63440e647495e0b008cfe1b8a8b2202d40
|
|
353f376277ad9b87e03c9ccbc1028c4b6d12e8ea Convert blockencodings.h to new serialization framework (Pieter Wuille)
e574fff53eec4a27c83b765cb69e31d8399047ea Add CustomUintFormatter (Pieter Wuille)
10633398f2dddf929d3f535aa48d138ad5e6c50f Add DifferenceFormatter (Russell Yanofsky)
56dd9f04c701aa3ac340e95065bf83de20373c8b Make VectorFormatter support stateful formatters (Russell Yanofsky)
3ca574cef0b4423f21b2c3efd8f5c9f71d52f219 Convert CCompactSize to proper formatter (Pieter Wuille)
Pull request description:
This is probably the most involved change in the sequence of changes extracted from #10785.
In order to implement the differential encoding of BIP152, this change changes `VectorFormatter` to permit a stateful sub-formatter, which is then used by `DifferenceFormatter`. A `CustomUintFormatter` is added as well to do the 48-bit serialization of short ids.
ACKs for top commit:
laanwj:
ACK 353f376277ad9b87e03c9ccbc1028c4b6d12e8ea, nice change
ryanofsky:
Code review ACK 353f376277ad9b87e03c9ccbc1028c4b6d12e8ea. Only changes since last review are suggested assert change and MASK->MAX rename
Tree-SHA512: 976618991a8be62ba0738725b7cfa166a56cde998ebf1031ba6f28557032f1577b666ac7ae25cd498c0e1e740108c3c56a342620b724df41d6cc9d8bdafac037
|
|
1891245e7318bf625bbf67aab08a79fc3e87b61d refactor: Cast ping values to double before output (Ben Woosley)
7a810b1d7a9d03818706dc94457dc3255f062796 refactor: Convert ping wait time from double to int64_t (Ben Woosley)
e6fc63ec7ee637a4b533e4d7b22def05e74e1dff refactor: Convert min ping time from double to int64_t (Ben Woosley)
b054c46977667953593819248c167545aa3e7a40 refactor: Convert ping time from double to int64_t (Ben Woosley)
Pull request description:
Alternative to #18252, see motivation there.
This changes `CNodeStats` to handle ping timestamps as their original incoming usec `int64_t` values until the time they need to be displayed.
ACKs for top commit:
vasild:
ACK 1891245
practicalswift:
ACK 1891245e7318bf625bbf67aab08a79fc3e87b61d -- patch looks correct
promag:
ACK 1891245e7318bf625bbf67aab08a79fc3e87b61d, added cast to double and also braces.
Tree-SHA512: 7cfcba941d9751b522b8c512c25da493338b444637bd0bb711b152d7d86b431ca0968956be3c844ee9dbfea25edab44a0de2afa44f2c9c0bf5b8df53eba66272
|
|
2455aa5d7f54befeade05795ed8f5dd89d01042a [rpc] changed MineBlocksOnDemand to IsMockableChain (Gloria Zhao)
Pull request description:
Change: Update the if statement in `setmocktime` to use `IsMockableChain` chainparams function (aka `m_is_mockable_chain`) instead of `MineBlocksOnDemand`
Rationale: It's a more appropriate check for whether or not chain is in RegTest, as [discussed](https://github.com/bitcoin/bitcoin/pull/18037#discussion_r376509388) in #18037
ACKs for top commit:
MarcoFalke:
ACK 2455aa5d7f54befeade05795ed8f5dd89d01042a 🙇
jonatack:
ACK 2455aa5d7f54befeade05795ed8f5dd89d01042a
Tree-SHA512: 1d8c8b7ff0b3c1bcbf5755194969b6664fe05a35003375ad08d18e34bcefd2df4f64d0e60078a10bbef3c8f469a9b9d07db467089b55c14cf532304bc965bffc
|
|
|
|
Note the divisor is a floating point literal so presumably
also floating point.
|
|
|
|
|
|
|
|
and update feature_asmap.py and test_runner.py
This commit moves the asmap init.cpp code from the end of "Step 12: start node"
to "Step 6: network initialization" to provide feedback on passing an -asmap
config arg much more quickly. This change speeds up the feature_asmap.py
functional test file from 60 to 5 seconds by accelerating the 2 tests that use
`assert_start_raises_init_error`.
Credit to Wladimir J. van der Laan for the suggestion.
|
|
and mention it is only available if the asmap config flag is set.
|
|
and remove redundant public declaration
|
|
- move asmap #includes to sorted positions in addrman and init (move-only)
- remove redundant quotes in asmap InitError, update test
- remove full stops from asmap logging to be consistent with debug logging,
update tests
|
|
and update the tests.
|
|
- allow passing an absolute file path to the -asmap config arg
- update the -asmap config help
- add a functional test in feature_asmap.py
|
|
and move to sorted position
|
|
fa6b061fc118995eec41766519a11bc0dd1a901d rpc: Auto-format RPCResult (MarcoFalke)
fa7d0503d320900e14c4d9bc016d65c7431070bb rpc: Move OuterType enum to header (MarcoFalke)
Pull request description:
This enforces most syntax rules of the RPCResult at compile time (or some at run time during unit and functional tests)
Apart from normalizing the syntax, by separating stylistic formatting from the structure, we could in theory directly generate the html for e.g. https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/
ACKs for top commit:
Sjors:
Indeed, re-ACK fa6b061fc118995eec41766519a11bc0dd1a901d
ajtowns:
ACK fa6b061fc118995eec41766519a11bc0dd1a901d -- skimmed code changes and differences to rpc help output
Tree-SHA512: 5b510b3aa0b7c7b9189a48c77593159409069f939145b9a00c5478e894cf65f994d44d633eb7bb7dbea40ee820645a2930976c24772379d96929002b120efa28
|
|
And ci script output.
Identified via test/lint/lint-spelling
|
|
aff2748f8aee72f03b5fb6f6f97f0d0f66391755 httpserver: use own HTTP status codes (Filip Gospodinov)
Pull request description:
Before, macros defined in `<event2/http.h>` have been used for some HTTP status codes. `<event2/http.h>` is included implicitly and the usage of its status code macros is inconsistent with the majority HTTP response implementations in this file.
Now, the `HTTPStatusCode` enum from `<rpc/protocol.h>` is consistently used for all HTTP response implementations.
ACKs for top commit:
practicalswift:
ACK aff2748f8aee72f03b5fb6f6f97f0d0f66391755 -- patch looks correct
laanwj:
ACK aff2748f8aee72f03b5fb6f6f97f0d0f66391755
Tree-SHA512: 6a7043488b88dcd584215d16b5f16f7bd668fe5553d31298d1beef134d2b0648aef81533014e34d1cd600baa36ee4e853f195352f4a00c866bd5ab1ff688bd50
|
|
1ef28b4f7cfba410fef524def1dac24bbc4086ca Make AnalyzePSBT next role calculation simple, correct (Gregory Sanders)
Pull request description:
Sniped test and alternative to https://github.com/bitcoin/bitcoin/pull/18220
Sjors documenting the issue:
```
A PSBT signed by ColdCard was analyzed as follows (see #17509 (comment))
{
"inputs": [
{
"has_utxo": true,
"is_final": false,
"next": "finalizer"
}
],
"estimated_vsize": 141,
"estimated_feerate": 1e-05,
"fee": 1.41e-06,
"next": "signer"
}
I changed AnalyzePSBT so that it returns "next": "finalizer" instead.
```
It makes it much clearer that the role has been decided before hitting the `calc_fee` block, and groups all state-deciding in one spot instead of 2.
Note that this assumes that PSBT roles are a complete ordering, which for now and in the future seems to be a correct assumption.
ACKs for top commit:
Sjors:
ACK 1ef28b4f7cfba410fef524def1dac24bbc4086ca, much nicer. Don't forget to document the bug fix.
achow101:
ACK 1ef28b4f7cfba410fef524def1dac24bbc4086ca
Empact:
ACK https://github.com/bitcoin/bitcoin/pull/18224/commits/1ef28b4f7cfba410fef524def1dac24bbc4086ca
Tree-SHA512: 22ba4234985c6f9c1445b14565c71268cfaa121c4ef000ee3d5117212b09442dee8d46d9701bceddaf355263fe25dfe40def2ef614d4f2fe66c9ce876cb49934
|
|
10efc0487c442bccb0e4a9ac29452af1592a3cf2 Templatize ValidationState instead of subclassing (Jeffrey Czyz)
10e85d4adc9b7dbbda63e00195e0a962f51e4d2c Remove ValidationState's constructor (Jeffrey Czyz)
0aed17ef2892478c28cd660e53223c6dd1dc0187 Refactor FormatStateMessage into ValidationState (Jeffrey Czyz)
Pull request description:
This removes boilerplate code in the subclasses which otherwise only
differ by the result type.
The subclassing was introduced in a27a295.
ACKs for top commit:
MarcoFalke:
ACK 10efc0487c442bccb0e4a9ac29452af1592a3cf2 🐱
ajtowns:
ACK 10efc0487c442bccb0e4a9ac29452af1592a3cf2 -- looks good to me
jonatack:
ACK 10efc048 code review, build/tests green, nice cleanup
Tree-SHA512: 765dd52dde7d49b9a5c6d99d97c96f4492673e2aed0b0604faa88db0308fa4500a26bf755cca0b896be283874096c215932e1110a2d01dc012cd36a5fce58a42
|
|
d36146009fb3fc9b9a772823b4df139a85173481 Drop unused mach time headers (Ben Woosley)
Pull request description:
Now that we're no longer special-casing clock usage for MacOS (see #17800), we're
not referencing anything defined in these headers.
Incidentally, this removes our last reference to the `__MACH__` system def. 🎉
ACKs for top commit:
jonasschnelli:
utACK d36146009fb3fc9b9a772823b4df139a85173481
fanquake:
ACK d36146009fb3fc9b9a772823b4df139a85173481 - thanks.
Tree-SHA512: 246045b0683a705ad034416e8ace2024e652026a6c0517b6797320e52fc18a6e111ec2e405ca40653bd1d6421bb7755232e8fec22651fff8e448eb7d5646a954
|
|
8888461f6814ae8b6221b02049fb9e1f69a5ff70 util: Fail to parse empty string in ParseMoney (MarcoFalke)
fab30b61eb51538a4db62e34f7133c44575b3fe9 util: Remove unused ParseMoney that takes a c_str (MarcoFalke)
Pull request description:
Supplying a fee rate or an amount on the command line as an empty string, which currently parses as `0` seems fragile and confusing. See for example the confusion in #18214.
Fixes #18214
ACKs for top commit:
Empact:
Code Review ACK https://github.com/bitcoin/bitcoin/pull/18225/commits/8888461f6814ae8b6221b02049fb9e1f69a5ff70
achow101:
ACK 8888461f6814ae8b6221b02049fb9e1f69a5ff70
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/18225/commits/8888461f6814ae8b6221b02049fb9e1f69a5ff70
Tree-SHA512: ac2d6b7fa89fe5809c34d5f49831042032591c34fb3c76908d72fed51e8bced41bf2b41dc1b3be34ee691a40463355649857a7a8f378709d38ae89503feb11c2
|
|
Now that we're no longer special-casing clock usage for MacOS, we're
not referencing anything defined in these headers.
|
|
16d6113f4faa901e248adb693d4768a9e5019a16 Refactor message transport packaging (Jonas Schnelli)
Pull request description:
This PR factors out transport packaging logic from `CConnman::PushMessage()`.
It's similar to #16202 (where we refactor deserialization).
This allows implementing a new message transport protocol like BIP324.
ACKs for top commit:
dongcarl:
ACK 16d6113f4faa901e248adb693d4768a9e5019a16 FWIW
ariard:
Code review ACK 16d6113
elichai:
semiACK 16d6113f4faa901e248adb693d4768a9e5019a16 ran functional+unit tests.
MarcoFalke:
ACK 16d6113f4faa901e248adb693d4768a9e5019a16 🙎
Tree-SHA512: 8c2f8ab9f52e9b94327973ae15019a08109d5d9f9247492703a842827c5b5d634fc0411759e0bb316d824c586614b0220c2006410851933613bc143e58f7e6c1
|
|
dc9305b6162ec615ff5fb2876e4f312051b543af random: don't special case clock usage on macOS (fanquake)
Pull request description:
`clock_gettime()`, `CLOCK_MONOTONIC` and `CLOCK_REALTIME` are all available for use on
macOS (now that we require macOS >=10.12 and build against 10.14). Use them rather than the [deprecated](https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/Mach/Mach.html) `mach_timespec_t` time API.
I mentioned the possibility for this change [in #17270](https://github.com/bitcoin/bitcoin/pull/17270#discussion_r346090606).
[master](1dbf3350c683f93d7fc9b861400724f6fd2b2f1d):
```bash
2019-12-23T20:49:43Z Feeding 216 bytes of dynamic environment data into RNG
2019-12-23T20:50:43Z Feeding 216 bytes of dynamic environment data into RNG
```
This PR:
```bash
2019-12-23T20:32:41Z Feeding 232 bytes of dynamic environment data into RNG
2019-12-23T20:33:42Z Feeding 232 bytes of dynamic environment data into RNG
```
~~Depends on #16392.~~ Merged.
ACKs for top commit:
laanwj:
ACK dc9305b6162ec615ff5fb2876e4f312051b543af
Tree-SHA512: 18c2f336ea628f9cf7339b817381d230a18893fd9c0351bf99a39ca6f45c5b0a20af9d599d48d6c09515627d5edafa91337c17f9f790264251d2cdcb3763bbd5
|
|
7bf4ce4f644bb7dac9b63172c656b5d599eedea3 refactor: test/bench: dedup SetupDummyInputs() (Sebastian Falbesoner)
Pull request description:
The only difference between `SetupDummyInputs()` in `test/transaction_tests.cpp` and the one in `bench/ccoins_caching.cpp` was the nValue amounts of the outputs, so we allow to pass those in an extra (fixed-size) array parameter.
ACKs for top commit:
MarcoFalke:
re-ACK 7bf4ce4f64, only change is schuffling includes 🚶
Empact:
ACK https://github.com/bitcoin/bitcoin/pull/18173/commits/7bf4ce4f644bb7dac9b63172c656b5d599eedea3
Tree-SHA512: e13643b2470f6b6ab429da0c0a8eebd4cb41e2ff2e421ef36f85fa4847bf4ea8aab88d59a01e94cac4c4eb85edb561463f02215b174c50b573ac6bbcc2bf98a3
|
|
The only difference between SetupDummyInputs() in test/transaction_tests.cpp
and the one in bench/ccoins_caching.cpp was the nValue amounts of the outputs,
so we allow to pass those in an extra (fixed-size) array parameter.
|
|
c72a11a1a030036eb1fe4472086a9733731961ce test: Add cost_of_change parameter assertions to bnb_search_test (Yancy Ribbens)
Pull request description:
If the `cost_of_change` variable is removed from the method body `SelectCoinsBnB`, there are currently no failing unit tests. This PR adds assertions about the behavior of the `cost_of_change`: If the cost of creating a change output is greater than what's leftover, then consume the output and create no change, otherwise, don't consume the output (no match found).
ACKs for top commit:
achow101:
ACK c72a11a1a030036eb1fe4472086a9733731961ce
Tree-SHA512: 613aa411df5e2911446e0e8bf3309336faaadf2d3c56e7d125b76454e7c6f9e4f5e8f0910dc6222282628e38cd8a4a7c56bb3d36b564a17f396b9b503ecc64c8
|
|
|
|
|
|
|
|
This removes boilerplate code in the subclasses which otherwise only
differ by the result type.
|
|
|
|
|
|
transport)
2f63ffd15caeb79867e56c8cedbe2c702952db9e tests: Add fuzzing harness for V1TransportDeserializer (P2P transport) (practicalswift)
Pull request description:
Add fuzzing harness for `V1TransportDeserializer` (P2P transport).
**Testing this PR**
Run:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/p2p_transport_deserializer
…
```
ACKs for top commit:
MarcoFalke:
ACK 2f63ffd15caeb79867e56c8cedbe2c702952db9e
Tree-SHA512: 8507d4a0414d16f1b8cc9649e3e638f74071dddc990d7e5d7e6faf77697f50bdaf133e49e2371edd29068a069a074469ef53148c6bfc9950510460b81d87646a
|
|
fab2527515e8db944ae044bea8580e2cd9414bcd test: Disable mockforward scheduler unit test for now (MarcoFalke)
Pull request description:
This should be a workaround to fix #18174 in the short run and buy us more time to investigate the issue while ci runs are green again :pray:
ACKs for top commit:
fanquake:
ACK fab2527515e8db944ae044bea8580e2cd9414bcd - be good to get Travis back.
laanwj:
ACK fab2527515e8db944ae044bea8580e2cd9414bcd
Tree-SHA512: 027e86b3dfec203a464e5bf528e9933c208c36633c2d4bfcdbc10da1799637a5d6ea0a63af33a4174fb1ad7115df631a4cb838f56e31f4cbd15498e1e9fdf9cc
|
|
for type-punning
0653939ac130eddffe40c53ac418bea305d3bf82 Add static_asserts to ser_X_to_Y() methods (Samer Afach)
be94096dfb0c4862e2314cbae4120d7360b08ef2 Fix a violation of C++ standard rules that unions cannot be switched. (Samer Afach)
Pull request description:
Type punning in C++ is not like C. As per the C++ standard, one cannot use unions to convert the bit type. A discussion about this can be found [here](https://stackoverflow.com/questions/25664848/unions-and-type-punning). In C++, a union is supposed to only hold one type at a time. It's intended to be used only as `std::variant`. Switching types is undefined behavior.
In fact, C++20 has a special casting function, called [`bit_cast`](https://en.cppreference.com/w/cpp/numeric/bit_cast) that solved this problem.
Why has it been working so far? Because some compilers tolerate using unions and switching types, like gcc. More information [here](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#Type-punning).
One important thing to mention is that performance is generally not affected by that memcpy. Compilers are smart enough to convert that to a memory cast when possible. But we have to do it the right way, otherwise, it's jut undefined behavior that depends on the compiler.
ACKs for top commit:
practicalswift:
ACK 0653939ac130eddffe40c53ac418bea305d3bf82
elichai:
ACK 0653939ac130eddffe40c53ac418bea305d3bf82
laanwj:
Code review ACK 0653939ac130eddffe40c53ac418bea305d3bf82
kristapsk:
ACK 0653939ac130eddffe40c53ac418bea305d3bf82
Tree-SHA512: f6e89de39fc964750429139bab6b5a1346f7060334b7afa020e315bdad8f8c195bce2b8a9e343f06e7fff175e2dfb1cdabfcb6fe405bea0febe4962f0cc62557
|
|
facb71576cd4d2e90fd03e09d29b42fa3d730e8c net: Remove forcerelay of rejected txs (MarcoFalke)
Pull request description:
This removes the code that supposedly handled the forced relay of txs from a permissioned peer that were rejected from our mempool. The removal should be fine, because it is dead code for the following reasons:
* While `RelayTransaction` enqueues the inv for all peers, the inv is never processed because it can not be found in the mempool. See https://github.com/bitcoin/bitcoin/blob/4a072330763b3ff2d1b5c5b8d30a9517873ac6de/src/net_processing.cpp#L3862-L3866
* Even if the peers we intended to send the inv to can somehow reply with a getdata to the never-received inv, they won't receive the tx as a reply because it was never added to the "relay memory" (`mapRelay`)
The dead code is (obviously) untested: https://marcofalke.github.io/btc_cov/total.coverage/src/net_processing.cpp.gcov.html#2574
This feature was (intentionally or accidentally) removed in 4d8993b3469915d8c9ba4cd3b918f16782edf0de, which was released in Bitcoin Core 0.13.0. So all currently supported versions of Bitcoin Core ship without this feature. I am not aware of any complaints about this feature or actual documented use-cases. So instead of reviving an unneeded feature, just remove the dead code.
ACKs for top commit:
hebasto:
ACK facb71576cd4d2e90fd03e09d29b42fa3d730e8c, locally running the unit and functional tests.
Tree-SHA512: bfceae6f2983c1510fa0649a9a63c343cbbc1c4ab3a3698039cccf454c81e58c8f5114b147ed42a1bc867da74c43a5b53764ab14f942e191b6f59079044108b5
|
|
|
|
|
|
|
|
(CBloomFilter + CRollingBloomFilter)
eabbbe409f397e97b1e6fad7385d9d1813ae2880 tests: Add fuzzing harness for rolling bloom filter class CRollingBloomFilter (practicalswift)
2a6a6ea0f5b97cba95b5678268d638c81764b9b1 tests: Add fuzzing harness for bloom filter class CBloomFilter (practicalswift)
Pull request description:
Add fuzzing harness for bloom filter classes (`CBloomFilter` + `CRollingBloomFilter`).
Test this PR using:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/bloom_filter
…
$ src/test/fuzz/rolling_bloom_filter
…
```
ACKs for top commit:
MarcoFalke:
ACK eabbbe409f397e97b1e6fad7385d9d1813ae2880 🤞
Tree-SHA512: 765d30bc52e3eb04dbd4d2b8f517387aa61312416e8fea3767250ef5c074e08641699019ee4600d42303de32f98379c20bfc0c0e60cb5154d0338088c1d29cb6
|
|
|
|
|
|
|
|
This is needed so that it can be used by RPCResult
Also,
* rename NAMED_ARG to NONE for generalization.
* change RPCArg constructors to initialize the members by moving values
|
|
5bad7921d0b33b62c0a59a478c2e8c869fc5e3b5 [test] PSBT RPC: check that bip32_derivs are present by default (Sjors Provoost)
29a21c90610aed88b796a7a5900e42e9048b990e [rpc] set default bip32derivs to true for psbt methods (Sjors Provoost)
Pull request description:
In https://github.com/bitcoin/bitcoin/pull/13557#pullrequestreview-135905054 I recommended not including bip32 deriviation by default in PSBTs:
> _Bit of a privacy issue_: let's say person A and B are about to spend from a multisig address, sending everything to person A. Person A gives their address to person B, their wallet wallet creates a PSBT, but doesn't sign it. Wallet A then calls `walletprocesspsbt` which signs it and _spontaneously adds the master_fingerprint and bip32 path_. Same issue with `walletcreatefundedpsbt`.
>
> Adding `bip32_derivs` should probably be opt-in.
In practice I find this default quite annoying because I forget it and end up with a confused hardware wallet.
More importantly, in the multisig example I provided, it's actually essential for the other side to know the derivation details (in addition to an xpub). This allows them to check that change is going to an address you can still co-sign for (because the spending policy is unchanged except for an index).
ACKs for top commit:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/17264/commits/5bad7921d0b33b62c0a59a478c2e8c869fc5e3b5
jonatack:
ACK 5bad7921d0 code review, built, ran tests, inspected/messed around with/pprinted values from the new tests. Thanks for adding the tests.
meshcollider:
utACK 5bad7921d0b33b62c0a59a478c2e8c869fc5e3b5
Tree-SHA512: 22ad71dda96856060a96758c4ae7aafa22d5e9efba30e0c8287c711e7579849bd72593cbc0f41a2e9e8821315d78bda04e848dbb006283b841b2795e2faebcfd
|
