[24.x] Backports

fanquake commented at 9:58 am on January 12, 2023: member

Backports:

fanquake added the label Backport on Jan 12, 2023

DrahtBot commented at 9:58 am on January 12, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	instagibbs, hebasto, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

fanquake commented at 10:28 am on January 12, 2023: member

Added #26234 & #26388 to fix the no-longer available Intel macOS box in the CI. Will rebase on top of #26880 when it’s merged.

maflcko referenced this in commit 5148145b35 on Jan 12, 2023

fanquake force-pushed on Jan 12, 2023

fanquake added this to the milestone 24.1 on Jan 19, 2023

wallet: fully migrate address book entries for watchonly/solvable wallets

Currently `migratewallet` migrates the address book (i.e. labels and
purposes) for watchonly and solvable wallets only in RAM, but doesn't
persist them on disk. Fix this by adding another loop for both of the
special wallet types after which writes the corresponding NAME and
PURPOSE entries to the database in a single batch.

Github-Pull: #26761
Rebased-From: d5f4ae7fac0bceb0c9ad939b9a4fbdb85da0bf95

342abfb3f4

test: wallet: check that labels are migrated to watchonly wallet

Github-Pull: #26761
Rebased-From: 730e14a317ae45fe871c8d6f44a51936756bbbea

cbcdafa471

wallet: Skip rescanning if wallet is more recent than tip

If a wallet has key birthdates that are more recent than the currrent
chain tip, or a bestblock height higher than the current tip, we should
not attempt to rescan as there is nothing to scan for.

Github-Pull: #26679
Rebased-From: 378400953424598fd78ccec5ba8cc38bc253c550

428dcd51e6

For feebump, ignore abandoned descendant spends

To be eligible for fee-bumping, a transaction must not have any
of its outputs (eg - change) spent in other unconfirmed transactions
in the wallet. However, this check should not apply to abandoned
transactions.

A new test case is added to cover this case.

Github-Pull: #26675
Rebased-From: f9ce0eadf4eb58d1e2207c27fabe69a5642482e7

5c824ac5e1

hash: add HashedSourceWriter

This class is the counterpart to CHashVerifier, in that it
writes data to an underlying source stream,
while keeping a hash of the written data.

Github-Pull: #26909
Rebased-From: da6c7aeca38e1d0ab5839a374c26af0504d603fc

91f83dbeb1

addrdb: Only call Serialize() once

The previous logic would call it once for serializing into the filestream,
and then again for serializing into the hasher. If AddrMan was changed
in between these calls by another thread, the resulting peers.dat would
be corrupt with non-matching checksum and data.
Fix this by using HashedSourceWriter, which writes the data
to the underlying stream and keeps track of the hash in one go.

Github-Pull: #26909
Rebased-From: 5eabb61b2386d00e93e6bbb2f493a56d1b326ad9

07397cdede

Add missing includes to fix gcc-13 compile error

Github-Pull: #26924
Rebased-From: fadeb6b103cb441e0e91ef506ef29febabb10715

7cf73dfed5

depends: fix systemtap download URL

Github-Pull: #26944
Rebased-From: d81ca6619a5d05472af7f59e36cd100dd04a3a01

cff67180b3

Correctly limit overview transaction list

The way that the main overview page limits the number
of transactions displayed (currently 5) is not
an appropriate use of Qt. If it's run with a DEBUG
build of Qt, it'll result in a segfault in certain
relatively common situations. Instead of artificially
limiting the rowCount() in the subclassed proxy
filter, we hide/unhide the rows in the displaying
QListView upon any changes in the sorted proxy filter.

Github-Pull: bitcoin-core/gui/pull/704
Rebased-From: 08209c039ff4ca5be4982da7a2ab7a624117ce1a

b7e242ecb3

Zero out wallet master key upon lock

When an encrypted wallet is locked (for instance via the
RPC `walletlock`), the docs indicate that the key is
removed from memory. However, the vector (with a secure
allocator) is merely cleared. This allows the key to persist
indefinitely in memory. Instead, manually fill the bytes with
zeroes before clearing.

Github-Pull: #27080
Rebased-From: 3a11adc7004d21b3dfe028b190d83add31691c55

64e7db6f4f

wallet: reuse change dest when recreating TX with avoidpartialspends

Github-Pull: #27053
Rebased-From: 14b4921a91920df25b19ff420bfe2bff8c56f71e

a62c541ae8

fanquake force-pushed on Feb 22, 2023

i2p: reuse created I2P sessions if not used

In the case of `i2pacceptincoming=0` we use transient addresses
(destinations) for ourselves for each outbound connection. It may
happen that we
* create the session (and thus our address/destination too)
* fail to connect to the particular peer (e.g. if they are offline)
* dispose the unused session.

This puts unnecessary load on the I2P network because session creation
is not cheap. Is exaggerated if `onlynet=i2p` is used in which case we
will be trying to connect to I2P peers more often.

To help with this, save the created but unused sessions and pick them
later instead of creating new ones.

Alleviates: https://github.com/bitcoin/bitcoin/issues/26754

Github-Pull: #26837
Rebased-From: b906b64eb76643feaede1da5987a0c4d466c581b

5027e93b6a

i2p: lower the number of tunnels for transient sessions

This will lower the load on the I2P network. Since we use one transient
session for connecting to just one peer, a higher number of tunnels is
unnecessary.

This was suggested in:
https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1365449401
https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1367356129

The options are documented in:
https://geti2p.net/en/docs/protocol/i2cp#options

A tunnel is unidirectional, so even if we make a single outbound
connection we still need an inbound tunnel to receive the messages sent
to us over that connection.

Alleviates: https://github.com/bitcoin/bitcoin/issues/26754

Github-Pull: #26837
Rebased-From: 801b405f85b413631427c2d8cc1f8447309ea5d8

29cdf42226

i2p: use consistent number of tunnels with i2pd and Java I2P

The default number of tunnels in the Java implementation is 2 and in the
C++ i2pd it is 5. Pick a mid-number (3) and explicitly set it in order
to get a consistent behavior with both routers. Do this for persistent
sessions which are created once at startup and can be used to open up
to ~10 outbound connections and can accept up to ~125 incoming
connections. Transient sessions already set number of tunnels to 1.

Suggested in:
https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1367356129
https://geti2p.net/en/docs/api/samv3

Alleviates: https://github.com/bitcoin/bitcoin/issues/26754

Github-Pull: #26837
Rebased-From: 3c1de032de01e551992975eb374465300a655f44

ab3bd457cd

wallet: Allow MigrateLegacyToDescriptor to take a wallet name

An overload of MigrateLegacyToDescriptor is added which takes the wallet
name. The original that took a wallet pointer is still available, it
just gets the name, closes the wallet, and calls the new overload.

Github-Pull: #26595
Reabsed-From: dbfa34540372033d95036a02b7025ddd33f540aa

648b06256d

rpc: Allow users to specify wallet name for migratewallet

Github-Pull: #26595
Rebased-From: 6bdbc5ff590de18dfb47c31190baad879f68fef7

50dd8b13df

wallet: Be able to unlock the wallet for migration

Since migration reloads the wallet, the wallet will always be locked
unless the passphrase is given. migratewallet can now take the
passphrase in order to unlock the wallet for migration.

Github-Pull: #26595
Rebased-From: 7fd125b27d48e410509f3009e2eb9fa5cd6729dd

ccc72fecd7

tests: Tests for migrating wallets by name, and providing passphrase

Github-Pull: #26595
Rebased-From: aaf02b5721a8b5d3d9280dc3146fa5e44ea671b6

debcfe313a

wallet, rpc: Update migratewallet help text for encrypted wallets

Github-Pull: #26595
Rebased-From: 9486509be65f09174a0cb50a337cac58a0c09de4

784a754aa4

fanquake requested review from stickies-v on Feb 27, 2023

instagibbs commented at 2:56 pm on February 27, 2023: member

ACK https://github.com/bitcoin/bitcoin/pull/26878/commits/784a754aa47ce10c6fd99c09cdfc76ee9bc91652

trivial backports as far as I can see

hebasto approved

hebasto commented at 4:47 pm on February 27, 2023: member

ACK 784a754aa47ce10c6fd99c09cdfc76ee9bc91652, I’ve made backporting locally and got a diff between my branch and this PR as follows:

 0--- a/test/functional/wallet_importdescriptors.py
 1+++ b/test/functional/wallet_importdescriptors.py
 2@@ -484,8 +484,8 @@ class ImportDescriptorsTest(BitcoinTestFramework):
 3         assert_equal(addr, 'bcrt1qp8s25ckjl7gr6x2q3dx3tn2pytwp05upkjztk6ey857tt50r5aeqn6mvr9') # Derived at m/84'/0'/0'/1
 4         change_addr = wmulti_pub.getrawchangeaddress('bech32') # uses change 2
 5         assert_equal(change_addr, 'bcrt1qp6j3jw8yetefte7kw6v5pc89rkgakzy98p6gf7ayslaveaxqyjusnw580c') # Derived at m/84'/1'/0'/2
 6-        assert(send_txid in self.nodes[0].getrawmempool(True))
 7-        assert(send_txid in (x['txid'] for x in wmulti_pub.listunspent(0)))
 8+        assert send_txid in self.nodes[0].getrawmempool(True)
 9+        assert send_txid in (x['txid'] for x in wmulti_pub.listunspent(0))
10         assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 999)
11 
12         # generate some utxos for next tests

which tracks down to 459cb637aca80f744a8399e84bc78fab60de0b5c from #26257 which, in turn, is not backported here.

trivial backports as far as I can see

https://github.com/bitcoin-core/gui/pull/704 and #27053 are not trivial to backport.

hebasto closed this on Feb 27, 2023

hebasto reopened this on Feb 27, 2023

achow101 commented at 6:08 pm on February 27, 2023: member

ACK 784a754aa47ce10c6fd99c09cdfc76ee9bc91652

glozow merged this on Feb 27, 2023

glozow closed this on Feb 27, 2023

fanquake deleted the branch on Feb 27, 2023

fanquake referenced this in commit 9e05de1d70 on Mar 13, 2023

brunoerg commented at 10:32 am on February 27, 2024: contributor

post merge ACK 784a754aa47ce10c6fd99c09cdfc76ee9bc91652

[24.x] Backports #26878

Reviews