Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 furszy commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1479743921)
yeah @pinheadmz, same as in the other issue. Agree on unifying all this issues into a single one. So progress updates aren't spread across the repository (I didn't even know that this existed before) and we can make reviewers calls if the sub-projects that are going toward this goal require review.
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1144975901)
I'll fix this.
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1144977982)
Ok, thanks.
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1479758216)
> but I think the commit message [should be updated](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1348929370) to better describe the bug, the fix, and the behaviour change introduced

Yes, I have that pending to do asap.
💬 ajtowns commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1479772800)
> wrt how the `TxValidationResult` types are used in `MaybePunishNodeForTx`, is something like this what `TX_RECENT_CONSENSUS_CHANGE` was intended for?

See #15141 where `RECENT_CONSENSUS_CHANGE` was introduced, and #11639 where it was discussed more (under the name `SOFT_FORK`). (Note that at that time, it applied to both blocks and txs, the validation results weren't separated until #15921 -- so some of the discussion there focuses more on blocks that violate recent soft fork rules rather t
...
💬 MarcoFalke commented on pull request "build: debug enable addrman consistency checks":
(https://github.com/bitcoin/bitcoin/pull/27300#issuecomment-1479775571)
Maybe there can be a `--enable-debug-symbols` (implied by `--enable-debug`) to set `-g` for the compiler?
💬 MarcoFalke commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1479777501)
ok, closing for now as duplicate. Let us know if it isn't and if this should be reopened.
MarcoFalke closed an issue: "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")"
(https://github.com/bitcoin/bitcoin/issues/27188)
💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1479793412)
For reference, the original release note in 0.18.0 for `blank` is as follows:

> `createwallet` now has an optional `blank` argument that can be used to create a blank wallet. Blank wallets do not have any keys or HD seed. They cannot be opened in software older than 0.18. Once a blank wallet has a HD seed set (by using `sethdseed`) or private keys, scripts, addresses, and other watch only things have been imported, the wallet is no longer blank and can be opened in 0.17.x. Encrypting a blank
...
💬 pinheadmz commented on issue "getchaintips doesn't mark headers-only chain as invalid":
(https://github.com/bitcoin/bitcoin/issues/8050#issuecomment-1479808811)
I'm looking into this, and I've written a functional test that reproduces the issue. One thing we could do is add some extra logic into `getchaintips` itself:

- Populate `setTips`
- Iterate through `setTips` and set `status` strings
- Then we could add this here:
- If the status is `"headers-only"`, walk that branch from tip to main chain
- If an ancestor with `BLOCK_FAILED_MASK` is found, reverse walk back to tip adding `BLOCK_FAILED_CHILD` flag and inserting into `m_dirty_blockindex
...
💬 achow101 commented on pull request "wallet: Migrate non-HD keys with single combo containing a list of keys":
(https://github.com/bitcoin/bitcoin/pull/26627#issuecomment-1479842340)
> Wouldn't #20018 address most performance concerns of having a large number of `DescriptorSPKM`s? Albeit at the cost of a slightly higher memory footprint, especially in this case.

Yes, but these approaches aren't mutually exclusive, and I'd like to get feedback on both.
💬 willcl-ark commented on pull request "build: debug enable addrman consistency checks":
(https://github.com/bitcoin/bitcoin/pull/27300#issuecomment-1479890611)
@mzumsande FYI this PR is equivalent to running with `-checkaddrman=1` which does use quite some resources as it does a full addrman consistency check on pretty much every addrman operation (Add, Good, Attempt, Get, Select, Connected etc.).

For me this manifests as saturating an entire core via a combination of the `b-msghand`, `b-net` and `b-opencon` threads as these all spawn consistency checks. My `bitcoind` process usually idles at ~5%, so to have one core @ 100% is a pretty big change,
...
💬 achow101 commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#issuecomment-1479917634)
ACK d7cc503843769b789dc5f031b4ddc75d555ba980
🚀 achow101 merged a pull request: "RPC: Fix fund transaction crash when at 0-value, 0-fee"
(https://github.com/bitcoin/bitcoin/pull/27271)
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1479975523)
@jamesob interesting that it didn't see a bigger speedup, but I guess it depends on a lot of other factors as well. How fast is your harddisk, and how much RAM does your computer have?
📝 ryanofsky opened a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302)
Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen:

- One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.con
...
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145197139)
That here would be `BOOST_TEST(block != b)`, because since `b` now has to come from the `::operator new` and not from the freelist
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145200479)
With the above change, if I'm not mistaken I don't think it's safe to check for `BOOST_TEST(block != b)` here too. This too calls `::operator new`, and because the previous memory was deallocated already, it might give out the same block of memory, depending on however malloc is implemented
👍 pinheadmz approved a pull request: "blockstorage: Adjust fastprune limit if block exceeds blockfile size"
(https://github.com/bitcoin/bitcoin/pull/27191)
ACK e930814fdb9261f180d5c436cefe52e9cf38fd67

I also wrote a test for this that freezes (infinite loop) on master but passes on this branch. Take it or leave it, I just wanted to see the mechanism work before acking:

https://github.com/pinheadmz/bitcoin/commit/c576efa6dc7c32981d3b2fb42c9c8573b8c2e293

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK e930814fdb9261f180d5c436cefe52e9cf38fd67
-----BEGIN PGP SIGNATURE-----

iQIzBAEB
...
💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1480012074)
It turns out that encrypting a blank legacy wallet also does not unset the blank flag nor does it generate a new seed.

The latest push removes the descriptor wallet changes and adds tests for the behavior when encrypting.