## Issue being fixed or feature implemented
Dashmate wanted a way to know if it is safe to restart the masternode.
This new RPC indicates the number of active DKG sessions, and the number
of blocks until next potential DKG.
## What was done?
Examples of responses:
`{'active_dkgs': 0, 'next_dkg': 22}`
## How Has This Been Tested?
`feature_llmq_rotation.py` was updated
## Breaking Changes
no
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
RPC `getassetunlockstatuses` is now accepting an extra optional
parameter `height`.
When a valid `height` is passed, then the RPC returns the status of
AssetUnlock indexes up to this specific block. (Requested by Platform
team)
## What was done?
Note that in order to avoid cases that can lead to deterministic result,
when `height` is passed, then the only `chainlocked` and `unknown`
outcomes are possible.
## How Has This Been Tested?
`feature_asset_locks.py` was updated.
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Trivial bug: `keypoolrefill` internally updates the status of the
progress bar each second.
```
m_storage.UpdateProgress(strMsg, static_cast<int>(dProgress));
```
However it can happen that one second is not enough time to make
significant progress and
`static_cast<int>(dProgress) = 0`.
Calling the function with `0` as progress opens a new progress bar and
this led to problems like #5730
## What was done?
trivially make sure to update the progress only if the parameter is >0
## How Has This Been Tested?
rpc does not spam anymore
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [X] I have performed a self-review of my own code
- [X] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
519cae8fd6e44aef3470415d7c5e12acb0acd9f4 gui: Delay interfaces::Node initialization (Russell Yanofsky)
102abff9eb6c267af64f2a3560712147d1896e13 gui: Replace interface::Node references with pointers (Russell Yanofsky)
91aced7c7e6e75c1f5896b7e3843015177f32748 gui: Remove unused interfaces::Node references (Russell Yanofsky)
e1336316250ab5cb0ed654b1e593378a6e0769ce gui: Partially revert #10244 gArgs and Params changes (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
This is a partial revert of https://github.com/bitcoin/bitcoin/pull/10244. It changes gui code to go back to using gArgs and Params() functions directly instead of using interfaces::Node to handle arguments.
These changes were originally pushed as part of https://github.com/bitcoin/bitcoin/pull/19461. Motivation is to support a new GUI process connecting to an already running node process. Details are explained in commit messages, but in addition to spawning a new bitcoin-node process, we want bitcoin-gui to connect to an existing bitcoin-node process. So for that reason it should be able to parse its own parameters, rather than rely on the node.
ACKs for top commit:
MarcoFalke:
re-ACK 519cae8fd6, only change is rebase and addressed nits of my previous review last week 🌄
Tree-SHA512: 9c339dd82ba78bcc7b887b84d872f35ccc7dfa3d271691e6eafe8a2048cbbe3bdde1e810ce33d0714d75d048c9de3470e9e9b6f8306a6047d1cb3548f6858dc8
241434200ec2067673d8522fee4f1228abfd8247 refactor: qt: Use vQueueNotifications.clear() (João Barbosa)
989e579d07bb5031639060b717f7a0be15d10e29 qt: Make transaction notification queue wallet specific (João Barbosa)
7b3b2303f44031c3545651858f697a495c3ea37a move-only: Define TransactionNotification before TransactionTablePriv (João Barbosa)
Pull request description:
Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet.
This means that some transactions can be missed if multiple wallets are loaded.
Fix this by having a queue for each wallet.
ACKs for top commit:
jonasschnelli:
utACK 241434200ec2067673d8522fee4f1228abfd8247
hebasto:
ACK 241434200ec2067673d8522fee4f1228abfd8247, I have reviewed the code and it looks OK, I agree it can be merged.
ryanofsky:
Code review ACK 241434200ec2067673d8522fee4f1228abfd8247. Only change is dropping one commit
Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
1afcd41a906e6417925e80578c0d850d269dc008 [net] Remove CombinerAll (John Newbery)
Pull request description:
This was introduced in 9519a9a4 for use with boost signals. Boost signals
have not been used in net since 8ad663c1, so this code is unused.
ACKs for top commit:
MarcoFalke:
review ACK 1afcd41a906e6417925e80578c0d850d269dc008
laanwj:
code review ACK 1afcd41a906e6417925e80578c0d850d269dc008
Tree-SHA512: a4313142afb88bf12f15abc4e717b3b0d0b40d2d5db2638494af3181e1cd680d7b036087050fc0e0dfe606228849a2e20ae85135908a9ebe8ff2130f163920e1
88df300f20da02060694cfe643e1c882efaa306c qt: Do not translate file extensions (Hennadii Stepanov)
Pull request description:
File extensions are untranslatable by their nature.
ACKs for top commit:
laanwj:
Concept and code review ACK 88df300f20da02060694cfe643e1c882efaa306c
Talkless:
tACK 88df300f20da02060694cfe643e1c882efaa306c, tested on Debian Sid with Qt 5.15.2. Tested all filters except for .psbt.
jarolrod:
re-ACK 88df300f20da02060694cfe643e1c882efaa306c
Tree-SHA512: 104d383543edcee8fb825f98d3b6669a7aaae2c74b6602f9bc407bf1c88be121ec535f2f9be87afa6ca775dc79865165f620553f6f6ab1d31a3f9ea93f7f9593
ac7ccd67d7f2b09e36dd57405f899e4698dd3d78 scripted-diff: Remove unused "What's This" button in dialogs on Windows (Hennadii Stepanov)
b6951483ecdd4409a0e1d492c93bcd4d823f039d qt: Add flags to prevent a "What's This" button on Windows OS (Hennadii Stepanov)
Pull request description:
Fix#74.
From [Qt docs](https://doc.qt.io/qt-5/qdialog.html#QDialog):
> The widget flags _f_ are passed on to the `QWidget` constructor. If, for example, you don't want a **What's This** button in the title bar of the dialog, pass `Qt::WindowTitleHint | Qt::WindowSystemMenuHint` in _f_.
Screenshot on Windows 10 (2004):
- master (3ba25e3bdde3464eed5d2743d68546e48b005544)
- this PR (e322fe7e19ac504272d14b9b4f9b28b13df888ed)
ACKs for top commit:
Bosch-0:
tACK ac7ccd67d7f2b09e36dd57405f899e4698dd3d78 Tested on Windows 10.0.18363 Build 18363.
promag:
Code review ACK ac7ccd67d7f2b09e36dd57405f899e4698dd3d78 but with some suggestions.
jonasschnelli:
utACK ac7ccd67d7f2b09e36dd57405f899e4698dd3d78
Tree-SHA512: f6750a17b7203106cb4db5870becba1cef6a505d4edcc710ba131338bd3aae051510627e62c9bcb8345a7f497c614709e11aeb8f6ae3ea85967bbce2a8c69e64
BACKPORT NOTICE
fixup psbt. all missing changes belongs to src/wallet/scriptpubkeyman.h/cpp ----- they are related to descriptor wallet!
-------------------
931dd4760855e036c176a23ec2de367c460e4243 Make lint-spelling.py happy (Glenn Willen)
11a0ffb29d1b4dcc55c8826873f340ab4196af21 [gui] Load PSBT from clipboard (Glenn Willen)
a6cb0b0c29d327d01aebb98b0504f317eb19c3dc [gui] PSBT Operations Dialog (sign & broadcast) (Glenn Willen)
5dd0c03ffa3aeaa69d8a3a716f902f450d5eaaec FillPSBT: report number of inputs signed (or would sign) (Glenn Willen)
9e7b23b73387600d175aff8bd5e6624dd51f86e7 Improve TransactionErrorString messages. (Glenn Willen)
Pull request description:
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.
This is based on Sjors' #17509, and depends on that PR going in first. (It effectively replaces the small "load PSBT" dialog from that PR with a more feature-rich one.)
Some notes:
* The way I display status information is maybe unusual (a status bar, rather than messageboxes.) I think it's helpful to have the information in it be persistent rather than transitory. But if people dislike it, I would probably move the "current state of the transaction" info to the top line of the main label, and the "what action just happened, and did it succeed" info into a messagebox.
* I don't really know much about the translation/localization stuff. I put tr() in all the places it seemed like it ought to go. I did not attempt to translate the result of TransactionErrorString (which is shared by GUI and non-GUI code); I don't know if that's correct, but it matches the "error messages in logs should be googleable in English" heuristic. I don't know whether there are things I should be doing to reduce translator effort (like minimizing the total number of distinct message strings I use, or something.)
* I don't really know how (if?) automated testing is applied to GUI code. I can make a list of PSBTs exercising all the codepaths for manual testing, if that's the right approach. Input appreciated.
ACKs for top commit:
instagibbs:
tested ACK 931dd47608
Sjors:
re-tACK 931dd4760855e036c176a23ec2de367c460e4243
jb55:
ACK 931dd4760855e036c176a23ec2de367c460e4243
achow101:
ACK 931dd4760855e036c176a23ec2de367c460e4243
Tree-SHA512: ade52471a2242f839a8bd6a1fd231443cc4b43bb9c1de3fb5ace7c5eb59eca99b1f2e9f17dfdb4b08d84d91f5fd65677db1433dd03eef51c7774963ef4e2e74f
5da96210fc2fda9fbd79531f42f91262fd7a9257 doc: release note for getpeerinfo last_block/last_transaction (Jon Atack)
cfef5a2c98b9563392a4a258fedb8bdc869c9749 test: rpc_net.py logging and test naming improvements (Jon Atack)
21c57bacda766a4f56ee75a2872f5d0f94e3901e test: getpeerinfo last_block and last_transaction tests (Jon Atack)
8a560a7d57cbd9f473d6a3782893a0e2243c55bd rpc: expose nLastBlockTime/TXTime as getpeerinfo last_block/transaction (Jon Atack)
02fbe3ae0bd91cbab2828cb7aa46f6493c82f026 net: add nLastBlockTime/TXTime to CNodeStats, CNode::copyStats (Jon Atack)
Pull request description:
This PR adds inbound peer eviction criteria `nLastBlockTime` and `nLastTXTime` to `CNodeStats` and `CNode::copyStats`, which then allows exposing them in the next commit as `last_transaction` and `last_block` Unix Epoch Time fields in RPC `getpeerinfo`.
This may be useful for writing missing eviction tests. I'd also like to add `lasttx` and `lastblk` columns to the `-netinfo` dashboard as described in https://github.com/bitcoin/bitcoin/pull/19643#issuecomment-671093420.
Relevant discussion at the p2p irc meeting http://www.erisian.com.au/bitcoin-core-dev/log-2020-08-11.html#l-549:
```text
<jonatack> i was specifically trying to observe and figure out how to test https://github.com/bitcoin/bitcoin/issues/19500
<jonatack> which made me realise that i didn't know what was going on with my peer conns in enough detail
<jonatack> i'm running bitcoin locally with nLastBlockTime and nLastTXTime added to getpeerinfo for my peer connections dashboard
<jonatack> sipa: is there a good reason why that (eviction criteria) data is not exposed through getpeerinfo currently?
<sipa> jonatack: nope; i suspect just nobody ever added it
<jonatack> sipa: thanks. will propose.
```
The last commit is optional, but I think it would be good to have logging in `rpc_net.py`.
ACKs for top commit:
jnewbery:
Code review ACK 5da96210fc2fda9fbd79531f42f91262fd7a9257
theStack:
Code Review ACK 5da96210fc2fda9fbd79531f42f91262fd7a9257
darosior:
ACK 5da96210fc2fda9fbd79531f42f91262fd7a9257
Tree-SHA512: 2db164bc979c014837a676e890869a128beb7cf40114853239e7280f57e768bcb43bff6c1ea76a61556212135281863b5290b50ff9d24fce16c5b89b55d4cd70
## Issue being fixed or feature implemented
`develop` can't sync from genesis on mainnet,
b8a086d5e7
broke it.
#5790 follow-up
## What was done?
Revive the old logic but using hardcoded block heights instead of
scanning via quorum manager.
## How Has This Been Tested?
Synced on mainnet/testnet, CI is happy
https://gitlab.com/UdjinM6/dash/-/pipelines/1148980046.
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
f32c408f3a0b7e597977df2bc2cdc4ae298586e5 Make sure unconfirmed parents are requestable (Pieter Wuille)
c4626bcd211af08c85b6567ef07eeae333edba47 Drop setInventoryTxToSend based filtering (Pieter Wuille)
43f02ccbff9b137d59458da7a8afdb0bf80e127f Only respond to requests for recently announced transactions (Pieter Wuille)
b24a17f03982c9cd8fd6ec665b16e022374c96f0 Introduce constant for mempool-based relay separate from mapRelay caching (Pieter Wuille)
a9bc5638031a29abaa40284273a3507b345c31e9 Swap relay pool and mempool lookup (Pieter Wuille)
Pull request description:
This implements the follow-up suggested here: https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627630111 . Instead of checking `setInventoryTxToSend`, maintain an explicit bloom filter with the 3500 most recently announced invs, and permit fetching any of these as long as they're in the relay pool or the mempool. In addition, permit relay from the mempool after just 2 minutes instead of 15.
This:
* Fixes the brief opportunity an attacker has to request unannounced invs just after the connection is established (pointed out by naumenkogs, see https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627627010).
* Guarantees that locally resubmitted invs after `filterInventoryKnown` rolls over can still be requested (pointed out by luke-jr, see https://github.com/bitcoin/bitcoin/pull/18861#discussion_r419695831).
It adds 37 KiB of filter per peer.
This is also a step towards dropping the relay pool entirely and always relaying from the mempool directly (see #17303), but that is still blocked by dealing properly with NOTFOUNDs (see #18238).
ACKs for top commit:
jnewbery:
reACK f32c408f3
jonatack:
re-ACK f32c408 per `git range-diff f7c19e8 2da7ee3 f32c408` and redid the following: code review, thought about motivation, DoS and privacy aspects, debug build to check for warnings after updating Clang from 6 to 11 since last review.
ajtowns:
re-ACK f32c408f3a0b7e597977df2bc2cdc4ae298586e5
Tree-SHA512: aa05b9fd01bad59581c4ec91836a52d7415dc933fa49d4c4adced79aa25aaad51e11166357e8c8b29fbf6021a7401b98c21b850b5d8e8ad773fdb5d6608e1e85
41dca087b73a3627107603694f5a982ea2a53189 [trivial] Extract connection type doc into file where it is used. (Amiti Uttarwar)
3069b56a456d98fca7c4a4ccd329581bd1f0b853 [doc] Improve help for getpeerinfo connection_type field. (Amiti Uttarwar)
Pull request description:
two commits addressing small followups from #19725
* first commit adds a clarification in the release notes that this field shouldn't be expected to be stable (suggested by sdaftuar in https://github.com/bitcoin/bitcoin/pull/19725#issuecomment-697421878)
* second commit moves the `CONNECTION_TYPE_DOC` object out of the header file to reduce the size of the binary (suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/19725#discussion_r495467895, he tested and found a decrease of 10kB)
ACKs for top commit:
achow101:
ACK 41dca087b73a3627107603694f5a982ea2a53189
laanwj:
Code review ACK 41dca087b73a3627107603694f5a982ea2a53189
Tree-SHA512: a555df978b4341fbe05deeb40a8a655f0d3c5c1c0adcc1737fd2cf61b204a5a24a301ca0c2b5a3616554d4abf8c57074d22dbda5a50d8450bc22c57679424985
a490d074b3491427afbd677f5fa635b910f8bb34 doc: Add anchors.dat to files.md (Hennadii Stepanov)
0a85e5a7bc8dc6587963e2e37ac1b087a1fc97fe p2p: Try to connect to anchors once (Hennadii Stepanov)
5543c7ab285e90256cbbf9858249e028c9611cda p2p: Fix off-by-one error in fetching address loop (Hennadii Stepanov)
4170b46544231e7cf1d64ac3baa314083be37502 p2p: Integrate DumpAnchors() and ReadAnchors() into CConnman (Hennadii Stepanov)
bad16aff490dcf87722fbfe202a869fb24c734e1 p2p: Add CConnman::GetCurrentBlockRelayOnlyConns() (Hennadii Stepanov)
c29272a157d09a8125788c1b860e89b63b4cb36c p2p: Add ReadAnchors() (Hennadii Stepanov)
567008d2a0c95bd972f4031f31647c493d1bc2e8 p2p: Add DumpAnchors() (Hennadii Stepanov)
Pull request description:
This is an implementation of #17326:
- all (currently 2) outbound block-relay-only connections (#15759) are dumped to `anchors.dat` file
- on restart a node tries to connect to the addresses from `anchors.dat`
This PR prevents a type of eclipse attack when an attacker exploits a victim node restart to force it to connect to new, probably adversarial, peers.
ACKs for top commit:
jnewbery:
code review ACK a490d074b3
laanwj:
Code review ACK a490d074b3491427afbd677f5fa635b910f8bb34
Tree-SHA512: 0f5098a3882f2814be1aa21de308cd09e6654f4e7054b79f3cfeaf26bc02b814ca271497ed00018d199ee596a8cb9b126acee8b666a29e225b08eb2a49b02ddd
faad92fe1c3cca9795226bd167130976930ddab8 test: Remove unused nVersion=1 in p2p tests (MarcoFalke)
Pull request description:
After commit ddefb5c0b759950942ac03f28c43b548af7b4033 nVersion is no
longer used in p2p logic when sending messages. Only when receiving
messages, but in this test no messages are received.
ACKs for top commit:
laanwj:
Code review ACK faad92fe1c3cca9795226bd167130976930ddab8
fanquake:
ACK faad92fe1c3cca9795226bd167130976930ddab8
Tree-SHA512: 9a7029187aaa5a7929a4a2199646131ff1ea72df6a855ce7022dd3bb2647dd525356dbc5e460c77007eebcdeab400a689db8cb77e8239af3b539c117a4e0d16e
b6834e312a6a7bb395ec7266bc9469384639df96 Avoid 'timing mishap' warnings when mocking (Pieter Wuille)
ec3916f40a3fc644ecbbaaddef6258937c7fcfbc Use mockable time everywhere in net_processing (Pieter Wuille)
Pull request description:
The fact that net_processing uses a mix of mockable tand non-mockable time functions made it hard to write functional tests for #19988.
I'm opening this as a separate PR as I believe it's independently useful. In some ways this doesn't go quite as far as it could, as there are now several data structures that could be converted to `std::chrono` types as well now. I haven't done that here, but I'm happy to reconsider that.
ACKs for top commit:
MarcoFalke:
ACK b6834e312a 🌶
jnewbery:
utACK b6834e312a6a7bb395ec7266bc9469384639df96
naumenkogs:
utACK b6834e3
Tree-SHA512: 6528a167c57926ca12894e0c476826411baf5de2f7b01c2125b97e5f710e620f427bbb13f72bdfc3de59072e56a9c1447bce832f41c725e00e81fea019518f0e
ddefb5c0b759950942ac03f28c43b548af7b4033 p2p: Use the greatest common version in peer logic (Hennadii Stepanov)
e084d45562b94827b3a7873895882fcaae9f4d48 p2p: Remove SetCommonVersion() from VERACK handler (Hennadii Stepanov)
8d2026796a6f7add0c2cda9806e759817d1eae6f refactor: Rename local variable nSendVersion (Hennadii Stepanov)
e9a6d8b13b0558b17cdafbd32fd2663b4138ff11 p2p: Unify Send and Receive protocol versions (Hennadii Stepanov)
Pull request description:
On master (6fef85bfa3cd7f76e83b8b57f9e4acd63eb664ec) `CNode` has two members to keep protocol version:
- `nRecvVersion` for received messages
- `nSendVersion` for messages to send
After exchanging with `VERSION` and `VERACK` messages via protocol version `INIT_PROTO_VERSION`, both nodes set `nRecvVersion` _and_ `nSendVersion` to _the same_ value which is the greatest common protocol version.
This PR:
- replaces two `CNode` members, `nRecvVersion` `nSendVersion`, with `m_greatest_common_version`
- removes duplicated getter and setter
There is no change in behavior on the P2P network.
ACKs for top commit:
jnewbery:
ACK ddefb5c0b759950942ac03f28c43b548af7b4033
naumenkogs:
ACK ddefb5c0b759950942ac03f28c43b548af7b4033
fjahr:
Code review ACK ddefb5c0b759950942ac03f28c43b548af7b4033
amitiuttarwar:
code review but untested ACK ddefb5c0b7
benthecarman:
utACK `ddefb5c`
Tree-SHA512: 5305538dbaa5426b923b0afd20bdef4f248d310855d1d78427210c00716c67b7cb691515c421716b6157913e453076e293b10ff5fd2cd26a8e5375d42da7809d
a8a64acaf32ac21feeb885671772282b531ef9a2 [BroadcastTransaction] Remove unsafe move operator (Amiti Uttarwar)
125c0381266e0e05a408f8e1818501ab73d29110 [p2p] Remove dead code (Amiti Uttarwar)
fc66d0a65cdc52a3b259effe0c29b5eafb1b5ff5 [p2p] Check for nullptr before dereferencing pointer (Adam Jonas)
cb79b9dbf4cd06e17c8c65b36bf15c3ea2641de4 [mempool] Revert unbroadcast set to tracking just txid (Amiti Uttarwar)
Pull request description:
Addresses some outstanding review comments from #18044
- reverts unbroadcast txids to a set instead of a map (simpler, communicates intent better, takes less space, no efficiency advantages of map)
- adds safety around two touchpoints (check for nullptr before dereferencing pointer, remove an inaccurate std::move operator)
- removes some dead code
Links to comments on wtxid PR: [1](https://github.com/bitcoin/bitcoin/pull/18044#discussion_r460495254) [2](https://github.com/bitcoin/bitcoin/pull/18044#discussion_r460496023) [3](https://github.com/bitcoin/bitcoin/pull/18044#discussion_r463532611)
thanks to jnewbery & adamjonas for flagging these ! !
ACKs for top commit:
sdaftuar:
utACK a8a64acaf32ac21feeb885671772282b531ef9a2
naumenkogs:
utACK a8a64acaf32ac21feeb885671772282b531ef9a2
jnewbery:
utACK a8a64acaf32ac21feeb885671772282b531ef9a2
Tree-SHA512: 7be669cb30cc17fb9e06b50e636ef7887c6a27354697987e4e4d38dba4b8f50e175647587430cd9bc3295bec01ce8b1e6639a50a4249d8fff9b1ca1b9ead3277
fb56d37612dea6666e7da73d671311a697570dae p2p: ensure inv is GenMsgTx before ToGenTxid in inv processing (John Newbery)
aa3621385ee66c9dde5c632c0a79fba3a6ea2d62 test: use CInv::MSG_WITNESS_TX flag in p2p_segwit (Jon Atack)
24ee4f01eadb870435712950a1364cf0def06e9f p2p: make gtxid(.hash) and fAlreadyHave localvars const (Jon Atack)
b1c855453bf2634e7fd9b53c4a76a8536fc9865d p2p: use CInv block message helpers in net_processing.cpp (Jon Atack)
acd66421671e42a58e8e067868e1ab86268e3231 [net processing] Change AlreadyHaveTx() to take a GenTxid (John Newbery)
5fdfb80b861e0de3fcf8a57163b3f52af4b2df3b [net processing] Change AlreadyHaveBlock() to take block_hash argument (John Newbery)
430e183b89d00b4148f0b77a6fcacca2cd948202 [net processing] Remove mempool argument from AlreadyHaveBlock() (John Newbery)
42ca5618cae0fd9ef97d2006b17d896bc58cc17c [net processing] Split AlreadyHave() into separate block and tx functions (John Newbery)
39f1dc944554218911b0945fff7e6d06f3dab284 p2p: remove nFetchFlags from NetMsgType TX and INV processing (Jon Atack)
471714e1f024fb3b4892a7a8b34a76b83a13fa19 p2p: add CInv block message helper methods (Jon Atack)
Pull request description:
Building on #19590 and the recent `wtxid` and `GenTxid` changes, this is a refactoring and cleanup PR to simplify and improve some of the net processing code.
Some of the diffs are best reviewed with `-w` to ignore spacing.
Co-authored by John Newbery.
ACKs for top commit:
laanwj:
Code review ACK fb56d37612dea6666e7da73d671311a697570dae
jnewbery:
utACK fb56d37612dea6666e7da73d671311a697570dae
vasild:
ACK fb56d3761
Tree-SHA512: ba39b58e6aaf850880a842fe5f6295e9f1870906ef690206acfc17140aae2ac854981e1066dbcd4238062478762fbd040ef772fdc2c50eea6869997c583e6a6d
## Issue being fixed or feature implemented
In the constructor of `WalletView` the pointer `masternodeListPage` is
initialized only if the setting `"fShowMasternodesTab"` is true.
```
if (settings.value("fShowMasternodesTab").toBool()) {
masternodeListPage = new MasternodeList();
addWidget(masternodeListPage);
}
```
When closing the wallet this check is done:
```
if (settings.value("fShowMasternodesTab").toBool() && masternodeListPage != nullptr) {
masternodeListPage->setClientModel(_clientModel);
}
```
it can happen that the `"fShowMasternodesTab"` becomes true after
calling the `WalletView` constructor and it leaves `masterNodeListPage`
uninitialized and this caused segfault on my pc.
And same happens for `governanceListPage`
## What was done?
The fix is trivial, I just set by default the two pointers to `nullptr`
## How Has This Been Tested?
wallet does not segfault anymore
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [X] I have performed a self-review of my own code
- [X] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
75122780e2c46505d977e24c5612dfa9442ab754 Increment input value sum only once per UTXO in decodepsbt (Andrew Chow)
Pull request description:
Refactors the UTXO processing of `decodepsbt` to extract the relevant `CTxOut` and handle the input amounts from that. This avoids double counting the input value.
Fixes#19516
ACKs for top commit:
sipa:
utACK 75122780e2c46505d977e24c5612dfa9442ab754
ryanofsky:
Code review ACK 75122780e2c46505d977e24c5612dfa9442ab754
Tree-SHA512: 004ec1597790a88a98098f1a26534d10ab0130a438dec0913522a529a8d7f18ad679948617dbcad6e541fbab5bcb2682aeed386b67746807c03b64d76ce5441d
ca3585a483ca5f6fc4cc54fd1530f89d13e5b7b0 [net/net processing] check banman pointer before dereferencing (John Newbery)
Pull request description:
Although we currently don't do this, it should be possible to create a
CConnman or PeerLogicValidation without a Banman instance. Therefore
always check that banman exists before dereferencing the pointer.
Also add comments to the m_banman members of CConnman and
PeerLogicValidation to document that these may be nullptr.
ACKs for top commit:
jonatack:
ACK ca3585a
theStack:
ACK ca3585a483
Tree-SHA512: 726401c8921b9a502029ead34ae797473a1bc359d6e4e58dcbe3e25b70dde40bb100723be467fd3e2bf418892c493911998226de19c9d529d72034e3be26be48
BACKPORT COMMENT
Central widget is null at this point in our implementation. Also, we have no issues with the toolbar which original PR was aimed to fix.
Code is not presented in this commit is DNM, merged only changes in qt/modaloverlay (except b4c1af9#diff-bd2b12135c597b047fffc3b7a8e86e4a965c86ec7763f6ea885b6d8f6f702d10R40-R42 which is unused)
--------------------
d0cc1f6df740e03ca0213a3754c3277b01ae2c05 qt: Disable toolbar when overlay is shown (Hennadii Stepanov)
e74cd2083d579b14b0b718aa36796f2bcf679600 qt, refactor: Cleanup ModalOverlay slots (Hennadii Stepanov)
Pull request description:
Keeping the main window toolbar activated while the modal overlay is shown could create the appearance of the non-responsive GUI.
Fixes#22.
---
On master (ca055885c631de8ac0ffe24be6b02835dbcc039d):
With this PR:
ACKs for top commit:
harding:
Tested ACK d0cc1f6df740e03ca0213a3754c3277b01ae2c05. Tested on Linux/X11 as much as I could given it's a pretty small change; seems like a nice improvement. I'm not experienced in Qt, but I don't see anything obviously problematic about the code.
jonatack:
ACK d0cc1f6 tested on Debian 5.7.6-1 (2020-06-24) x86_64 GNU/Linux
LarryRuane:
ACK d0cc1f6df740e03ca0213a3754c3277b01ae2c05 tested on Ubuntu 18.04.4 LTS
Tree-SHA512: e371b34231c01e77118deb100e0f280ba1cdef54e317f7f7d6ac322598bda811bd1bfe3035e90d87f8267f4f5d2095d34a8136911159db63694fd1b1b11335a1
f20b359bb9dabc7be11c3e3319e435aa42a8f0f5 cli: reduce DefaultRequestHandler memory allocations (Jon Atack)
Pull request description:
per https://github.com/bitcoin/bitcoin/pull/16439#discussion_r443957125. Simpler code, fewer allocations. No change of behavior. The code has good test coverage in `interface_bitcoin_cli.py`.
ACKs for top commit:
MarcoFalke:
review ACK f20b359bb9dabc7be11c3e3319e435aa42a8f0f5
fjahr:
Code review ACK f20b359
Tree-SHA512: 745eab44dfdcc485ca2bbc1db8b8d364cbd3cf94982e46e033745ce05ab617c15320ee55da7fb930d365f4d26b172049d5f5efcf0b6d3af5b0a28185bdb93ea8
e8a2822119233ade0de84f791a9e92918a3d6896 [net] Don't try to take cs_inventory before deleting CNode (John Newbery)
3556227ddd3365cfac43b307204d73058b2943f0 [net] Make cs_inventory a non-recursive mutex (John Newbery)
344e831de54f7b864f03a90f6cb19692eafcd463 [net processing] Remove PushBlockInventory and PushBlockHash (John Newbery)
Pull request description:
- Remove PushBlockInventory() and PushBlockHash(). These are one-line functions that can easy be inlined into the calling code. Doing so also allows us to eliminate the one place that cs_inventory is recursively locked.
- Make cs_inventory a nonrecursive mutex
- Remove a redundant TRY_LOCK of cs_inventory when deleting CNode.
ACKs for top commit:
sipa:
utACK e8a2822119233ade0de84f791a9e92918a3d6896
MarcoFalke:
ACK e8a2822119233ade0de84f791a9e92918a3d6896 🍬
hebasto:
re-ACK e8a2822119233ade0de84f791a9e92918a3d6896
Tree-SHA512: dbc721d102cdef7b5827a8f2549daf8b54f543050266999a7ea56c9f36618565b71e31ce0beb1209ba2db43d15388be173355a03fb6db8ad24e2475b145050bd
84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow)
46004790588c24174a0bec49b540d158ce163ffd psbt: always put a non_witness_utxo and don't remove it (Andrew Chow)
5279d8bc07d601fe6a67ad665fbc7591fe73c7de psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow)
72f6bec1da198764d4648a10a61c485e7ab65e9e rpc: show both UTXOs in decodepsbt (Andrew Chow)
Pull request description:
Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition.
Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper.
Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs.
As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs.
ACKs for top commit:
Sjors:
utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators)
ryanofsky:
Code review re-ACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to
meshcollider:
utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3
Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
56f9dba015c592b8925795012e3061a710070a27 Only relay IPv4, IPv6, Tor addresses (Pieter Wuille)
79f3d9b932bf62b90995bce1cf4b0b1f0152d26d Mention BIP155 in doc/bips.md (Pieter Wuille)
Pull request description:
This:
* Documents BIP155 support in doc/bips.md
* Restricts addrv2 relay to IPv4, IPv6, and Tor addresses. Relaying addresses in ranges that no network software has support for seems like a gratuitous spam vector.
ACKs for top commit:
jonatack:
ACK 56f9dba015c592b8925795012e3061a710070a27
naumenkogs:
ACK 56f9dba
hebasto:
ACK 56f9dba015c592b8925795012e3061a710070a27, verified both links.
Tree-SHA512: f0a2072b3d84a05cdbc7b961c18d7322a2e7260517f5306599ff52d8c728f9167de0a59a6d66cb95d84d69f3028680ce8bd05dab0db8c4f97938a287e5ce9631
af57766182013e17c23245671a33463f754ccd28 Fix misleading error message: Clean stack rule (sanket1729)
Pull request description:
Error messages in clean stack is misleading as it lets the user believe that there are extra
elements on the stack which is incorrect if the stack is empty.
Let me know if this requires additional test.
ACKs for top commit:
instagibbs:
re-ACK af57766182
gzhao408:
reACK af57766182
theStack:
re-ACK af57766182013e17c23245671a33463f754ccd28
darosior:
re ACK af57766182013e17c23245671a33463f754ccd28
Tree-SHA512: 88e77416e220b080246fec368f5552a891d102d072b7bee62ac560d5e31c4a8c2ee9cbe569740b253e9df177d21dc788d10d856b2a542ab47761bb81698e4082
fa7e407b504bc60c77341f02636ed9d6a4b53d79 Do not pass chain params to CheckForStaleTipAndEvictPeers twice (MarcoFalke)
Pull request description:
`PeerManager` already keeps a reference to the chain params as a member variable. No need to pass it in once again as a function parameter.
ACKs for top commit:
naumenkogs:
utACK fa7e407b504bc60c77341f02636ed9d6a4b53d79
jnewbery:
code review ACK fa7e407b504bc60c77341f02636ed9d6a4b53d79
epson121:
Code review ACK fa7e407b504bc60c77341f02636ed9d6a4b53d79
Tree-SHA512: 640c2d8adf9f1d54d0bfbdf81989064be2f5ba4b534d07d42258b372dc130f7b9c3fd087c7d28f0439678d124127f5d6f82f3139b1766f59f5ed661e7ac2a923
20c9e035543892e322c7134e89eb33115678bb30 gui: Call setWalletActionsEnabled(true) only for the first wallet (Hennadii Stepanov)
Pull request description:
On master (a78742830aa35bf57bcb0a4730977a1e5a1876bc) there is a bug:
- open an encrypted wallet; please note that the "Encrypt Wallet..." menu item is disabled that is expected:
- then open any other wallet; note that the "Encrypt Wallet..." menu item gets enabled that is wrong:
This PR fixes this bug.
ACKs for top commit:
jonasschnelli:
Tested ACK 20c9e035543892e322c7134e89eb33115678bb30 - I could reproduce the issue on master and have verify that this PR fixes it.
achow101:
ACK 20c9e035543892e322c7134e89eb33115678bb30
Tree-SHA512: 2c9ab94bde8c4f413b0a95c05bf3a1a29f5910e0f99d6639a11dd77758c78af25b060b3fecd78117066ef15b113feb79870bc1347cc04289da915c00623e5787
0d9d2a1f7c26dc9c7b233ea8c3182fe1f8936bca Only update the updateSmartFeeLabel once in sync (Jonas Schnelli)
Pull request description:
Calling `updateSmartFeeLabel` and therefore `estimateSmartFee` is pointless during IBD.
GUI freezes appear because `estimateSmartFee` competes with `processBlock` for the `m_cs_fee_estimator` lock leading to multiple seconds of blocking the GUI thread in `updateSmartFeeLabel`.
ACKs for top commit:
ryanofsky:
Code review ACK 0d9d2a1f7c26dc9c7b233ea8c3182fe1f8936bca. Clever fix. Didn't test but I remember I could reproduce the startup issue easily before by putting a sleep in estimateSmartFee.
promag:
Code review ACK 0d9d2a1f7c26dc9c7b233ea8c3182fe1f8936bca.
hebasto:
ACK 0d9d2a1f7c26dc9c7b233ea8c3182fe1f8936bca, tested on Linux Mint 20 (x86_64) with `QT_FATAL_WARNINGS=1` and `-debug=qt`.
Tree-SHA512: 85ec2266f06ddd7b523e24d2a462f10ed965d5b4d479005263056f81b7fe49996e1568dafb84658af406e9202ed3bfa846d59c10bb951e0f97cee230e30fafd5
7984c39be11ca04460883365e1ae2a496aaa6c0e test framework: serialize/deserialize inv type as unsigned int (Jon Atack)
407175e0c2bc797599ebd9c0a1f2ec89ad7af136 p2p: change CInv::type from int to uint32_t (Jon Atack)
Pull request description:
Fixes UBSan implicit-integer-sign-change issue per https://github.com/bitcoin/bitcoin/pull/19610#issuecomment-680686460.
Credit to Crypt-iQ for finding and reporting the issue and to vasild for the original review suggestion in https://github.com/bitcoin/bitcoin/pull/19590#pullrequestreview-455788826.
Closes#19678.
ACKs for top commit:
laanwj:
ACK 7984c39be11ca04460883365e1ae2a496aaa6c0e
MarcoFalke:
ACK 7984c39be11ca04460883365e1ae2a496aaa6c0e 🌻
vasild:
ACK 7984c39be
Tree-SHA512: 59f3a75f40ce066ca6f0bb1927197254238302b4073af1574bdbfe6ed580876437be804be4e47d51467d604f0d9e3a5875159f7f2edbb2351fdb2bb9465100b5
752e6ad5336d5af0db9fe16d24c0c6aa25b74a3f Protect localhost and block-relay-only peers from eviction (Suhas Daftuar)
Pull request description:
Onion peers are disadvantaged under our eviction criteria, so prevent eventual
eviction of them in the presence of contention for inbound slots by reserving
some slots for localhost peers (sorted by longest uptime).
Block-relay-only connections exist as a protection against eclipse attacks, by
creating a path for block propagation that may be unknown to adversaries.
Protect against inbound peer connection slot attacks from disconnecting such
peers by attempting to protect up to 8 peers that are not relaying transactions
but have provided us with blocks.
Thanks to gmaxwell for suggesting these strategies.
ACKs for top commit:
laanwj:
Code review ACK 752e6ad5336d5af0db9fe16d24c0c6aa25b74a3f
Tree-SHA512: dbf089c77c1f747aa1dbbbc2e9c2799c628028b0918d0c336d8d0e5338acedd573b530eb3b689c7f603a17221e557268a9f5c3f585f204bfb12e5d2e76de39a3
6d1f51343cf11b07cd401fbd0c5bc3603e185a0e [rpc] fundrawtransaction, walletcreatefundedpsbt lock manually selected coins (Sjors Provoost)
Pull request description:
When using `fundrawtransaction` and `walletcreatefundedpsbt` with `lockUnspents`, it would only lock automatically selected coins, not manually selected coins. That doesn't make much sense to me if the goal is to prevent accidentally double-spending yourself before you broadcast a transaction.
Note that when creating a transaction, manually selected coins are automatic "unlocked" (or more accurately: the lock is ignored). Earlier versions of this PR introduced an error when a locked coin is manually selected, but this idea was abandoned after some discussion. An application that uses this RPC should either rely on automatic coin selection (with `lockUnspents`) or handle lock concurrency itself with manual coin selection. In particular it needs to make sure to avoid/pause calls with automatic coin selection between calling `lockunspent` and the subsequent spending RPC.
See #7518 for historical background.
ACKs for top commit:
meshcollider:
Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e
fjahr:
Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e
Tree-SHA512: 8773c788d92f2656952e1beac147ba9956b8c5132d474e0880e4c89ff53642928b4cbfcd1cb3d17798b9284f02618a8830c93a9f7a4733e5bded96adff1d5d4d
10b7a6d532148f880568c529e61a6d7edc7c91a9 refactor: make txmempool interface use GenTxid (Pieter Wuille)
5c124e17407a5b5824fec062b73a03a1030fa28c refactor: make FindTxForGetData use GenTxid (Pieter Wuille)
a2bfac893549e2d62708d8cda7071b4fe9750a2d refactor: use GenTxid in tx request functions (Pieter Wuille)
e65d115b725640eefb3bfa09786447816f7ca9cc test: request parents of orphan from wtxid relay peer (Anthony Towns)
900d7f6c075fd78e63503f31d267dbc16b3983d9 p2p: enable fetching of orphans from wtxid peers (Pieter Wuille)
9efd86a908cf09d9ddbadd3195f202635117d505 refactor: add GenTxid (=txid or wtxid) type and use it for tx request logic (Pieter Wuille)
d362f19355b36531a4a82094e0259f7f3db500a7 doc: list support for BIP 339 in doc/bips.md (Pieter Wuille)
Pull request description:
This is based on https://github.com/bitcoin/bitcoin/pull/18044#discussion_r450687076.
A new type `GenTxid` is added to protocol.h, which represents a tagged txid-or-wtxid. The tx request logic is updated to use these instead of uint256s, permitting per-announcement distinguishing of txid/wtxid (instead of assuming that everything we want to request from a wtxid peer is wtx). Then the restriction of orphan-parent requesting to non-wtxid peers is lifted.
Also document BIP339 in doc/bips.md.
ACKs for top commit:
jnewbery:
Code review ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9
jonatack:
ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9
ajtowns:
ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9 -- code review. Using gtxid to replace the is_txid_or_wtxid flag for the mempool functions is nice.
naumenkogs:
utACK 10b7a6d
Tree-SHA512: d518d13ffd71f8d2b3c175dc905362a7259689e6022a97a0b4f14f1f9fdd87475cf5af70cb12338d1e5d31b52c12e4faaea436114056a2ae9669cb506240758b
This backport is marked as full, not partial, but it has only refactorings
and non-witness related changes.
Included commits are:
- test: Update test framework p2p protocol version to 70016
- Rename AddInventoryKnown() to AddKnownTx()
- Add support for tx-relay via wtxid
This adds a field to CNodeState that tracks whether to relay transactions with
that peer via wtxid, instead of txid. As of this commit the field will always
be false, but in a later commit we will add a way to negotiate turning this on
via p2p messages exchanged with the peer.
- Just pass a hash to AddInventoryKnown
Since it's only used for transactions, there's no need to pass in an inv type.
- Add wtxid to mempool unbroadcast tracking
## Issue being fixed or feature implemented
For platform needs `getrawtransactionmulti` will help to reduce amount
of rpc calls for sake of performance improvement.
## What was done?
Implemented new RPC, basic functional test, release note.
## How Has This Been Tested?
On testnet:
```
> getrawtransactionmulti '{"000000abbe61a4d9b9356cb1d7deb1132d0b444a62869e71c2f3aa8ce2361359":["6e3ef19a3f955ac75a1f84dae60d42bbe11548ef54e37033ff2d91b3c4a09e9c", "415d5fafd5ee24ada8b99c36df339785a3066170c0dca6bb1aa6a5b96cf51e35"], "0":["ec7090f01c0e9b6e29d3be8810b12c780d2fb34372a53b231ce18bb7d2f1e8b0"]}'
> getrawtransactionmulti '{"000000abbe61a4d9b9356cb1d7deb1132d0b444a62869e71c2f3aa8ce2361359":["6e3ef19a3f955ac75a1f84dae60d42bbe11548ef54e37033ff2d91b3c4a09e9c", "415d5fafd5ee24ada8b99c36df339785a3066170c0dca6bb1aa6a5b96cf51e35"], "0":["ec7090f01c0e9b6e29d3be8810b12c780d2fb34372a53b231ce18bb7d2f1e8b0"]}' true
```
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: pasta <pasta@dashboost.org>
8888bd43c100f9f0ca1122fcc896fb7b999d61c6 Remove redundant nLastTry check (MarcoFalke)
00001e57fe74c061aa9cbc72b07252335cb566e0 Remove redundant nTime checks (MarcoFalke)
Pull request description:
Split out from https://github.com/bitcoin/bitcoin/pull/24697 because it makes sense on its own.
ACKs for top commit:
dergoegge:
re-ACK 8888bd43c100f9f0ca1122fcc896fb7b999d61c6
naumenkogs:
utACK 8888bd43c100f9f0ca1122fcc896fb7b999d61c6
Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
292828cd7744ec7eadede4ad54aa2117087c5435 [test] Test addr cache for multiple onion binds (dergoegge)
3382905befd23364989d941038bf7b1530fea0dc [net] Seed addr cache randomizer with port from binding address (dergoegge)
f10e80b6e4fbc151abbf1c20fbdcc3581d3688f0 [net] Use ConnectedThroughNetwork() instead of GetNetwork() to seed addr cache randomizer (dergoegge)
Pull request description:
The addr cache id randomizer is currently supposed to be seeded with the network of the inbound connection and the local socket (only the address is used not the port): a8098f2cef/src/net.cpp (L2800-L2804)
For inbound onion connections `CNode::addr.GetNetwork()` returns `NET_UNROUTABLE` and `CNode::addrBind` is set to `127.0.0.1:<onion bind port>`. This results in the same addr cache for all inbound connections on 127.0.0.1 binds.
To avoid the same addr cache across all onion and other 127.0.0.1 binds, we should seed the addr cache randomizer with the correct network for inbound onion connections (using `CNode::ConnectedThroughNetwork()`) as well as the port of `CNode::addrBind`.
ACKs for top commit:
sipa:
utACK 292828cd7744ec7eadede4ad54aa2117087c5435
mzumsande:
Code Review ACK 292828cd7744ec7eadede4ad54aa2117087c5435
naumenkogs:
utACK 292828cd7744ec7eadede4ad54aa2117087c5435
Tree-SHA512: d0be13bab6bc121c2926d4b168687f6c2ed4ce0c9dd19be71eb4886adeba8afc3daacdc4e232a0ba3b03a89d69b618abc5595b69abd1ad0c476d825bc6ea1f9f
151009cf76f3f1adc17630d5370bf019be127373 qt, wallet, refactor: Drop unused `WalletModel::PaymentRequestExpired` (Hennadii Stepanov)
Pull request description:
The `PaymentRequestExpired` value in the `WalletModel::StatusCode` enumeration has been unused since bitcoin/bitcoin#17165.
ACKs for top commit:
furszy:
ACK 151009cf, no usage for it.
kristapsk:
cr ACK 151009cf76f3f1adc17630d5370bf019be127373, checked that `PaymentRequestExpired` is not referenced anywhere else.
Tree-SHA512: c2ea3443af5d369ca294d79559869f688aaa806b91ffe0090f3b34638a8377ec2f11d6f5c09cc2d11ab55035850237e60e992acba671097a6642c6bb9e709273
fa27ee88edbf696b7eef2efbfcf1446ec522fd85 Get time less often in AddrManImpl::ResolveCollisions_() (MarcoFalke)
Pull request description:
First commit split out from https://github.com/bitcoin/bitcoin/pull/24697
ACKs for top commit:
sipa:
utACK fa27ee88edbf696b7eef2efbfcf1446ec522fd85
fanquake:
ACK fa27ee88edbf696b7eef2efbfcf1446ec522fd85
Tree-SHA512: 40c8594d2a5ce02a392ac5f9f120c24c6bcd495b0bcc901fd6064dde9f6123cd109504cee7b612a9555b70cfd7759cbd6cd496d007bb374c27610d01b464191c
7036cf52aa080d2a63993c2298555252d507dd2f Delete UpdatePackagesForAdded at beginning of addPackageTxs. (KevinMusgrave)
Pull request description:
In `CreateNewBlock` (in miner.cpp), `inBlock` is cleared before `addPackageTxs`, so `inBlock` will be empty in the first call to `UpdatePackagesForAdded`. I saw this brought up in these [PR review club logs](https://bitcoincore.reviews/24538) and there didn't seem to be a definitive answer for why the call is necessary. There's also an [old PR](https://github.com/bitcoin/bitcoin/pull/10200) where this change was going to be applied, but it got closed.
If `addPackageTxs` can be called when `inBlock` is not empty, then maybe a test should be added for that case. All the tests seem to pass with this deletion.
ACKs for top commit:
glozow:
utACK 7036cf52aa080d2a63993c2298555252d507dd2f
Tree-SHA512: 9e757b71b9035f68a0c6fef229b8cd83f1bdbe23f05bb02cc1bab8c3c177805b388bceb2bb1f0bce354791ccb29f351a6c51979b96ffe4d9fc6c978f83e36afc
