Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582254)
Done
πŸ’¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582474)
Done
πŸ’¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187585622)
I just changed the error message and adopted the diff (seems so much better to check whether this is a `nullptr`):
```diff
- if (!output_connection_direction) {
+ if (output_connection_direction == nullptr) {
+ // Only NetWhitebindPermissions() should pass a nullptr.
```
πŸ’¬ hebasto commented on pull request "refactor: Remove unused GetTimeMillis":
(https://github.com/bitcoin/bitcoin/pull/27594#issuecomment-1538588861)
Concept ACK.
πŸ’¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187591763)
Added right before:
```diff
+ # Whitelist peers to speed up tx relay / mempool sync
```
πŸ’¬ furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187594094)
If `CreateCoins` generates a coin that is already inside any of the results, `AddInputs` will throw a runtime error, which the fuzzing framework will detect as a failure when it is not. It is part of the class boundaries.

Same apply for the fuzz `Merge` function commit.
πŸ’¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187595794)
Just added it to make it clearer
πŸ’¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1538611034)
Force-pushed addressing @jonatack's review.

- Moved `InitializePermissionFlags` out of `CConnman`
- Made `TryParsePermissionFlags` static
- Added more test coverage https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186367052
- Improved documentation
πŸ’¬ brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187606760)
> If CreateCoins generates a coin that is already inside any of the results, AddInputs will throw a runtime error, which the fuzzing framework will detect as a failure when it is not. It is part of the class boundaries.

Yes, that's why it calls `CreateCoins` twice and with different "utxos pool". Is there any risk yet?
πŸ’¬ Sjors commented on pull request "Update best_header inside Connect/DisconnectTip":
(https://github.com/bitcoin/bitcoin/pull/26260#issuecomment-1538638453)
I deployed this patch on the two ForkMonitor (mirror) nodes that frequently use `invalidateblock` and `reconsiderblock`. It's cherry-picked on top of the v25.0rc1 and v24.1rc2 tags. Typically we get about one error message per week where the block it rewinds to is not the one we expected. We'll see if that stops.
πŸ’¬ furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187625261)
No ok. The reason why this doesn't fails is the test global increasing `next_locktime` which ensures that all UTXOs receive a different hash even when they have the same data.
So, nvm. A comment wouldn't hurt but nothing serious.
πŸ’¬ achow101 commented on pull request "Switch hardened derivation marker to h":
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1538744152)
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
⚠️ fjahr opened an issue: "Node stuck with repeated "Cache size exceeds total space" log message"
(https://github.com/bitcoin/bitcoin/issues/27599)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Bug report from IRC `bitcoin-core-dev`: The node stopped updating the chain tip while printing "Cache size exceeds total space" multiple times per second. Node had to be restarted after "stale tip" messages, then resumed to work normally.

Start of conversation: https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-08#920380;
User logs: https://jb55.com/s/58818ccccfb21d95.txt

###
...
πŸ’¬ fjahr commented on issue "Node stuck with repeated "Cache size exceeds total space" log message":
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-1538754014)
@jb55 just repeating the open questions from IRC here:
- Do you have a custom `maxmempool` setting?
- Do you have the last UpdateTip line your node saw before the cache size messages? the end of those UpdateTip lines shows how big your coins cache was at the end of block validation (@sdaftuar )
- Did this happen around the time when we had the 2 block reorg ~20h ago, i.e. around 788685-788688 (not sure what timezone you are in with those logs)?
- Can you say which version and OS you are on
...
πŸ’¬ jamesob commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1538759099)
CI's passing after a silent conflict in the rebase. I've added a link to @Sjors' snapshot torrent in the PR description.

No known outstanding issues here; ready for testing!
πŸ’¬ achow101 commented on issue "GCC 13: `-Wdangling-reference` output":
(https://github.com/bitcoin/bitcoin/issues/26926#issuecomment-1538762824)
Getting these errors with gcc 13.1.1
⚠️ achow101 reopened an issue: "GCC 13: `-Wdangling-reference` output"
(https://github.com/bitcoin/bitcoin/issues/26926)
New warnings currently emitted by the shortly-to-be-released GCC 13.
This is building master (eebc24bfc6d2d809952e27c7fe269452f319455f), using GCC `gcc (GCC) 13.0.0 20230115 (Red Hat 13.0.0-0)`:
```bash
external_signer.cpp: In static member function β€˜static bool ExternalSigner::Enumerate(const std::string&, std::vector<ExternalSigner>&, std::string)’:
external_signer.cpp:33:25: warning: possibly dangling reference to a temporary [-Wdangling-reference]
33 | const UniValue& error =
...
πŸš€ achow101 merged a pull request: "Switch hardened derivation marker to h"
(https://github.com/bitcoin/bitcoin/pull/26076)
πŸ’¬ fjahr commented on issue "Node stuck with repeated "Cache size exceeds total space" log message":
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-1538784739)
> @jb55 just repeating the open questions from IRC here:
>
> * Do you have a custom `maxmempool` setting?
> * Do you have the last UpdateTip line your node saw before the cache size messages? the end of those UpdateTip lines shows how big your coins cache was at the end of block validation (@sdaftuar )
> * Did this happen around the time when we had the 2 block reorg ~20h ago, i.e. around 788685-788688 (not sure what timezone you are in with those logs)?
> * Can you say which version and
...
πŸ’¬ brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187734637)
Got it, thanks.