💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074935902)
nit in a29101b56541d4bf72fcf69460e8eb2a8cc33979: I guess you wanted to leave some of those for follow-ups, but the `doc/descriptors.md` one seems in-scope?

```
$ git grep -i 'legacy wallet' 1c0d89358e12fc871e99c8304d5cb50965cf7b9d doc/descriptors.md doc/managing-wallets.md src/bitcoin-wallet.cpp src/wallet src/script/ test/
1c0d89358e12fc871e99c8304d5cb50965cf7b9d:doc/descriptors.md:- `importmulti` takes as input descriptors to import into a legacy wallet
1c0d89358e12fc871e99c8304d5cb509
...

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074950668)
(same commit): remove

```
test/sanitizer_suppressions/tsan:race:BerkeleyBatch
```

?

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074924915)
(same commit): Forgot to remove `deprecatedrpc=create_bdb`?

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074946960)
Yeah, I meant that flushing isn't used by any chain client and I don't think there is much need to keep the dead code, but can be done in a follow-up.

💬 metasmile commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2853693834)
The main arguments supporting the removal of the OP_RETURN size limit appear to be as follows:

- If a transaction includes a large amount of data, it will naturally incur significant fees, which serve as a de facto limitation.
However, it must be seriously considered that the block space can still be abused by certain entities or groups who are indifferent to high fees.

- It has been argued that due to the currently open parameters, OP_RETURN has already been used in a way that is effecti
...

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074994436)
follow-up nit in 18a1bd87813326ac2b1df43b8d37e5815f1ec033: Could convert those to descriptor wallets?

💬 willcl-ark commented on pull request "fees: document non-monotonic estimation edge case":
(https://github.com/bitcoin/bitcoin/pull/31080#issuecomment-2853762080)
> @willcl-ark I guess you'll have to push the requested change, or reply to it, or close this pull?

Sure @maflcko, I took the suggestion and rebased.

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2075023514)
Also:

```
1c0d89358e12fc871e99c8304d5cb50965cf7b9d:src/wallet/rpc/wallet.cpp: {RPCResult::Type::STR, "format", "the database format (bdb or sqlite)"},

1c0d89358e12fc871e99c8304d5cb50965cf7b9d:test/functional/tool_wallet.py: def assert_is_bdb(self, filename):

1c0d89358e12fc871e99c8304d5cb50965cf7b9d:test/lint/lint-locale-dependence.py: "src/wallet/bdb.cpp:.*DbEnv::strerror", # False positive
```

And possibly:

```
sh-5.2$ git grep 'DeleteRecords('
...

✅ maflcko closed an issue: "Intermittent failure in tool_wallet.py in self.assert_tool_output('', '-wallet=salvage', 'salvage') : assert_equal(p.poll(), 0) ; AssertionError: not(3221226505 == 0)"
(https://github.com/bitcoin/bitcoin/issues/30894)

💬 maflcko commented on issue "Intermittent failure in tool_wallet.py in self.assert_tool_output('', '-wallet=salvage', 'salvage') : assert_equal(p.poll(), 0) ; AssertionError: not(3221226505 == 0)":
(https://github.com/bitcoin/bitcoin/issues/30894#issuecomment-2853782378)
For reference, BDB removal https://github.com/bitcoin/bitcoin/pull/31250 was merged (for 30.0)

✅ maflcko closed an issue: "importaddress failed, Only legacy wallets are supported by this command."
(https://github.com/bitcoin/bitcoin/issues/29772)

💬 maflcko commented on issue "importaddress failed, Only legacy wallets are supported by this command.":
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2853789616)
> or closing this as a duplicate of https://github.com/bitcoin/bitcoin/issues/30175

Let's move the discussion to one place

✅ maflcko closed an issue: "DB_RUNRECOVERY: Fatal error, run database recovery"
(https://github.com/bitcoin/bitcoin/issues/29026)

💬 maflcko commented on issue "DB_RUNRECOVERY: Fatal error, run database recovery":
(https://github.com/bitcoin/bitcoin/issues/29026#issuecomment-2853804340)
For reference, BDB removal https://github.com/bitcoin/bitcoin/pull/31250 was merged (for 30.0)

So I presume this was fixed as part of that pull

👍 willcl-ark approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2817471142)
reACK 7238f242d78f3d71834764031d7904d19525bab3

Changes since my last ACK (excl. the rebase) look OK to me.

💬 fanquake commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#discussion_r2075054496)
Please don't add triples with trailing versions numbers, as it's just something else to be "bumped". You can leave the windows example as-is.

💬 maflcko commented on issue "Networking tests fail on emulated big-endian systems":
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2853828012)
> I couldn't find the timeout value for:

The test framework has a timeout factor. You can simply increase it to avoid having to manually adjust each timeout. See `./bld-cmake/test/functional/test_runner.py --help | grep factor`

💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075066106)
I think "deprecated" is clear enough, no need to commit to a timeline.

💬 maflcko commented on issue "Networking tests fail on emulated big-endian systems":
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2853841237)
> the `interface_bitcoin_cli` tests are likely also related to the IPv6 big-endian failures:

What are the exact steps to reproduce? Ideally, starting from a fresh install? I guess it happens on macOS and on the Hetzner box equally?

💬 maflcko commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2853854910)
Looks like there wasn't any activity for more than half a year. Also the workaround https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407190445 may still work, so I guess this can be closed?