💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1280960230)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1271094067

> [332e847](https://github.com/bitcoin/bitcoin/commit/332e847c9ec0241efd9681eee3b03ff819aaddc3) tuple not needed per iwyu in tidy ci https://cirrus-ci.com/task/6540065057275904?logs=ci#L20325, while touching could add the others

Thanks, updated includes

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1280960587)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1277588274

> nit: would it be more idiomatic to use `has_value()` here?

I don't think it's more idiomatic but the suggestion seems fine so I adopted it.

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1280961601)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1271096350

> [332e847](https://github.com/bitcoin/bitcoin/commit/332e847c9ec0241efd9681eee3b03ff819aaddc3) per iwyu in https://cirrus-ci.com/task/6540065057275904?logs=ci#L20305

Thanks, updated includes

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1280960940)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1277584643

> Would it make sense to rename this to `emplace`, to keep the interface in line with `optional` and `expected`?

This isn't an actually an emplace method, it's an assignment method. An emplace method for a `Result<T>` object would take arguments that would be accepted by one of `T`'s constructors, and construct a new `T` object in place with them.

By contrast, this method doesn't accept arguments that could be forw
...

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1280961487)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1276417417

> It strikes me as unfortunate that the move constructor cannot move the failure values across different result value types, meaning the failure needs to be passed in as a separate argument. At the same time the user would be allowed to return a `{std::move(result), util::Error{Untranslated("str error")}`, potentially without the user noticing that this will not move the failure value ~and instead initialize a `Monostate
...

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1280960343)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1277642391

> nit: would `m_error` or (`m_error_info`) be a more appropriate name? E.g. in `bool()`, the meaning of `!m_error` is much more intuitive at first sight compared to `!m_info`, I think? (i.e. it's not really clear what it means to "not have info", whereas "not have error" is clear).

Calling it `m_error` would be misleading in later commits. The purpose of this field isn't to indicate the presence of an error. It happen
...

💬 achow101 commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#discussion_r1281126061)
`OP_VER` can be included here too:

```suggestion
opcode == OP_VERNOTIF ||
(fExec && opcode == OP_VER))
```

💬 TheCharlatan commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1281161914)
> WriteImpl, ssKey and ssValue are all CDBBatch class members, I'm not sure passing these as parameters makes sense? And if so, I think ssKey should be const.

This gave me pause too, but it felt clearer to me this way. Let me know if you think passing them arguments here is more confusing.

> At first sight, I'm not entirely sure why ssKey and ssValue are CDBBatch members when they get resized and cleared in every operation, so perhaps cleaning that up is the better way to go, but I don't l
...

💬 TheCharlatan commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#issuecomment-1661120330)
Thank you for the review @stickies-v,

Updated e8a3a0a0d761c8544e20ffc3ee3d4c1029c44518 -> 8c4481ed3713931247e4cedcb5733d3598050eb7 ([cleaveLeveldbHeaders_1](https://github.com/TheCharlatan/bitcoin/tree/cleaveLeveldbHeaders_1) -> [cleaveLeveldbHeaders_2](https://github.com/TheCharlatan/bitcoin/tree/cleaveLeveldbHeaders_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/cleaveLeveldbHeaders_1..cleaveLeveldbHeaders_2))
* Addressed @stickies-v's [comment](https://github.com/bitcoin/b
...

💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#issuecomment-1661125511)
Rebased 5720741c14e617ed338f64361b23f4e66ec99e07 -> 7721ab9139013d70ef0f058754fabc5aaeda2246 ([kernelReturnOnAbort_2](https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort_2) -> [kernelReturnOnAbort_3](https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelReturnOnAbort_2..kernelReturnOnAbort_3))

* Fixed conflict with https://github.com/bitcoin/bitcoin/pull/27746

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1281197388)
Updated, and added explanation in the commit message.

```diff
- case BCLog::LogFlags::NONE: // this case is normally never hit
- return "";
+ case BCLog::LogFlags::NONE:
+ Assume(false);
```

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1661161279)
Updated per `git diff 93b387f 82aa619` to address review feedback.

🤔 mzumsande reviewed a pull request: "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)"
(https://github.com/bitcoin/bitcoin/pull/28175#pullrequestreview-1557910458)
> The reality is that going forward it'll be essentially impossible to avoid contributions that may include output from AI/LLMs, just because (in almost all cases) it'll be impossible to tell, unless the author makes it apparent.

Maybe making it apparent is part of the problem. There is no requirement to state publicly which technical tools were involved in a contribution, so for now it might be best if everyone would just use their favourite LLM helpers silently (as, I am sure, many contribu
...

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1281205133)
Thank you @fanquake for the feedback and having a look. Updated to commit 88211f2b7e12542ba5e65bba4e3398c9592430ea per the diff in https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1661161279, replacing the comment with clearer code along with added explanation in the commit message.

💬 jonatack commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1661175302)
Concept ACK

💬 Daniel600 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1661182837)
Your research is not thorough and reaches an incorrect conclusion.
As stated many times - we service payment processors and some merchants directly  - Coinspaid services multiple merchants and process a significant amount of BTC they are a well known and active in the space - as I provided back in December 2022 a email from Max the CEO of Coinspaid confirming their use of 0-conf as well as providing there cluster addresses to validate there deposit flows see here again - https://lists.linuxfoun
...

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-1661192251)
Updated 1de05ef9190202f04f8cbe7746a47cbd66ab540c -> 08f5febfc571220043436bbec96a326beebdee22 ([`pr/bresult2.36`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.36) -> [`pr/bresult2.37`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.37), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult2.36..pr/bresult2.37)) replacing `util::Messages` with `util::MoveMessages` to work around clang-tidy error `bugprone-use-after-move` (https://cirrus-ci.com/task/665702225123737
...

💬 jonatack commented on issue "Intermittent CI failure "fee too high" in wallet_send.py --descriptors ":
(https://github.com/bitcoin/bitcoin/issues/25164#issuecomment-1661246977)
Seen in `wallet_send.py --legacy-wallet`

https://cirrus-ci.com/task/6115877712560128?logs=ci#L3429

```
node0 2023-08-01T23:17:59.093565Z [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=testmempoolaccept user=__cookie__
test 2023-08-01T23:17:59.094000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-p
...

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1661327327)
On Tue, Aug 01, 2023 at 03:28:31PM -0700, Daniel Lipshitz wrote:
> Your research is not thorough and reaches an incorrect conclusion.
> As stated many times - we service payment processors and some merchants directly  - Coinspaid services multiple merchants and process a significant amount of BTC they are a well known and active in the space - as I provided back in December 2022 a email from Max the CEO of Coinspaid confirming their use of 0-conf as well as providing there cluster addresses to v
...

⚠️ Bitnee opened an issue: "Berkley Database Errors"
(https://github.com/bitcoin/bitcoin/issues/28197)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [X] I still think this issue should be opened here

### Report

Attempting Restoration from 2016 year Wallet

Two Errors (many duplicate wallets supposedly)

BerleyDatabase: Can't open database {} (duplicates fileid 9770020000001b00539d761d0000000000000000 from {.dat}
&
PRUNED NODES {internally solved}

any support is held in much appreciations