Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ mzumsande commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1132967989)
Nice idea! I think that would work - I could do a full signet sync with it.

I'm still a bit undecided on general grounds, i.e. if it's okay to add more logic to accommodate a test-only mode that doesn't make sense for anything else but easier testing of pruning in the functional and unit tests. One could also argue that if no one but a developer would ever have a reason to use it, it'd be ok to just tell them more clearly not to create large blocks alongside using this option and abort otherw
...
πŸ’¬ amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1132968822)
oh interesting, I thought that the compiler optimizes away these sort of named-variables-used-shortly-after patterns, but maybe it's different for `std::string`? I took it to compiler explorer to see if I could observe the allocation- https://godbolt.org/z/GG78zv3KT. are the calls to `std::allocator<char>` what you are referring to?
⚠️ Lokoluis opened an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/27241)
Concept ACK

_Originally posted by @fanquake in https://github.com/bitcoin/bitcoin/issues/27233#issuecomment-1464352514_
βœ… fanquake closed an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/27241)
:lock: fanquake locked an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/27241)
πŸ’¬ fanquake commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#issuecomment-1464875015)
Tested 308ea03d95786db674e8cce3e78a56b498cf119a rebased on master (c7f1d95f52883d7087b74f45f5e4ce1100d51149) and I'm no-longer seeing the issue in #27229: `wallet_importdescriptors.py --descriptors | βœ“ Passed | 652 s`
πŸš€ fanquake merged a pull request: "doc: update broken str util reference links on developer-notes"
(https://github.com/bitcoin/bitcoin/pull/27220)
βœ… fanquake closed an issue: "Bug: ArgsManager::ReadSettingsFile can return false even when it does load settings"
(https://github.com/bitcoin/bitcoin/issues/22638)
πŸš€ fanquake merged a pull request: "util: fix argsman dupe key error"
(https://github.com/bitcoin/bitcoin/pull/27236)
πŸ’¬ fanquake commented on pull request "refactor: Consistently use context args over gArgs in node/interfaces":
(https://github.com/bitcoin/bitcoin/pull/27239#issuecomment-1464879171)
@TheCharlatan note that you've re-ACK'd the wrong commit hash.
πŸš€ fanquake merged a pull request: "refactor: Consistently use context args over gArgs in node/interfaces"
(https://github.com/bitcoin/bitcoin/pull/27239)
πŸ’¬ MarcoFalke commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1133071807)
> @MarcoFalke : Do you have an opinion on this, or do you remember past discussions

Both no :)
πŸ’¬ pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1133072579)
1. What I'm asserting here is that if a position is passed in, the function called "save block to disk" does not save a block to disk. By using incorrect data I can easily prove that it is not actually written to disk.
2. The call to `AddBlock` you highlight here confuses and concerns me a bit - since we know a `known` block won't be written, why are we updating the metadata at all?
⚠️ theStack opened an issue: "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2"
(https://github.com/bitcoin/bitcoin/issues/27242)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Building with the `--with-experimental-kernel-lib` configuration option currently leads to linker failures on OpenBSD 7.2. The following compiler/linker/build-tool versions are used:
- clang 13.0.0
- lld 13.0.0
- ccache 4.6.3
- GNU make 4.3
- autoconf 2.71
- automake 1.16.5

### Expected behaviour

-

### Steps to reproduce

```
$ ./configure --enable-suppress-external-warnings --wi
...
πŸ’¬ theStack commented on pull request "test: add coverage for sigop limit policy (`-bytespersigop` setting)":
(https://github.com/bitcoin/bitcoin/pull/27171#issuecomment-1464905433)
> @theStack could open a followup addressing anything here?
>
> > (Doesn't need to be in this PR) it could be worth checking that this vsize calculation is used for anc/desc limits as well?
>
> > for reference, usually fAccurate is set for witness scripts sigop counts, but omitting the k and n in the CMS imitates fAccurate=false.

Yes, I'm planning to do a follow-up next week, extending the checks on ancestor/descendant limits is definitely a good idea! Regarding Marco's explanation w.r.
...
πŸ’¬ davidgumberg commented on pull request "Improve address decoding errors":
(https://github.com/bitcoin/bitcoin/pull/26514#issuecomment-1464915896)
Maybe add a message for not-yet-understood witness versions and a test case?

```diff
if (version == 1 && data.size() == WITNESS_V1_TAPROOT_SIZE) {
static_assert(WITNESS_V1_TAPROOT_SIZE == WitnessV1Taproot::size());
WitnessV1Taproot tap;
std::copy(data.begin(), data.end(), tap.begin());
return tap;
}
+
+ if (version > 1 && version <= 16) {
+ error_str = "Invalid or unsupported Bech32 address witness version."
+ return CNoDestination();
+ }
+

...
πŸ’¬ sipa commented on pull request "Improve address decoding errors":
(https://github.com/bitcoin/bitcoin/pull/26514#issuecomment-1464916288)
@davidgumberg That would be incorrect; sending to future witness versions is perfectly legal. It might make sense in the UI to give a warning, but we can't outlaw it (as it'd break future upgradability).
πŸ’¬ davidgumberg commented on pull request "Improve address decoding errors":
(https://github.com/bitcoin/bitcoin/pull/26514#issuecomment-1464918507)
@sipa Thanks! I misunderstood.
πŸ’¬ mzumsande commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1133090332)

> 2\. The call to `AddBlock` you highlight here confuses and concerns me a bit - since we know a `known` block won't be written, why are we updating the metadata at all?

Because the block index LevelDB database (saved in `blocks/index`) is being wiped at the beginning of a reindex and needs to be rebuilt from scratch as we look at each block. Part of this database is meta info about each blockfile ([see bitcoin wiki](https://en.bitcoin.it/wiki/Bitcoin_Core_0.11_(ch_2):_Data_Storage#Block_in
...