👍 dergoegge approved a pull request: "msvc: Compile `test\fuzz\bitdeque.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29983#pullrequestreview-2031510928)
utACK 774359b4a96d2724dc70f900cb71e084a77164da

💬 stickies-v commented on pull request "rpc: Remove index-based Arg accessor":
(https://github.com/bitcoin/bitcoin/pull/29997#discussion_r1584915042)
> I think this was called `check_positional` because ...

Indeed, that was the rationale.

> so maybe a name like `check_args`

or `check_equivalence`? (no big deal either way ofc)

🤔 ryanofsky reviewed a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906#pullrequestreview-2031518836)
Updated 9552619961049d7673f84c40917b0385be70b782 -> 6a8b2befeab25e4e92d8e947a23e78014695e06c ([`pr/saferesult.8`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.8) -> [`pr/saferesult.9`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.9), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/saferesult.8..pr/saferesult.9)) tweaking variables to avoid shadowing

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1584914512)
re: https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581580725

> I'm happy to go with any suggestion. I don't see shadowing as a problem in all cases, so this is just the best way to write the code that I could think of.

Not sure why I couldn't think of a good way to write this without shadowing last week. But added `_retry` suffixes now so the code should be clearer and shadowing should be gone :sunglasses:

💬 glozow commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1584921222)
Should this be happening when `duplicate_inputs` is true (we'd be intentionally re-adding copies of the inputs we used)? Maybe I'm interpreting `duplicate_inputs` incorrectly but I thought that just means "we're ok with duplicates" not "we will add a copy of an outpoint every time we use it"

💬 glozow commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1584925110)
Approach-wise, sorry to bikeshed, but did you consider just making outpoints a set instead of a vector?

💬 maflcko commented on pull request "rpc: Remove index-based Arg accessor":
(https://github.com/bitcoin/bitcoin/pull/29997#discussion_r1584932568)
I am happy to review a follow-up, if there is a better name. But I think the current name is perfectly fine and accurate.

In any case, if the legacy code is removed, the test will go away as well.

💬 sr-gi commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1584955259)
Yeah, that may reduce the boilerplate and, given this is a test, performance shouldn't matter much

💬 sr-gi commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1584957576)
The way I've understood `duplicate_inputs` is: "We are OK with using the same input more than once in a transaction". The way the test was built, re-using inputs across transactions was always an option, so I took that for granted.

💬 stickies-v commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#issuecomment-2085563710)
re-ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c

✅ glozow closed a pull request: "txorphanage: add size accounting, use wtxids, support multiple announcers"
(https://github.com/bitcoin/bitcoin/pull/28481)

💬 glozow commented on pull request "txorphanage: add size accounting, use wtxids, support multiple announcers":
(https://github.com/bitcoin/bitcoin/pull/28481#issuecomment-2085578562)
Closing for now. The first commit is superseded by #30000, and the rest will come after `TxDownloadManager` refactoring

🤔 furszy reviewed a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906#pullrequestreview-2031664498)
re-ACK https://github.com/bitcoin/bitcoin/commit/6a8b2befeab25e4e92d8e947a23e78014695e06c

💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-2085584460)
@hebasto thanks for the review, I addressed your comments

💬 ajtowns commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2085586513)
> > > I think most of the criticism of this PR from AJ is saying that the changes here are unnecessary, not really that they are harmful,

> > I said "As I've already said, I think `m_log.SetCategory(NET); m_log.Debug("connected to foo")` is much worse than `LogDebug(NET, "connected to foo")`." Does that really not come across as me saying they're harmful? I really don't think I'm being **that** unclear. Anyway, I'll bow out, I'm already sorry for touching this.

> I think it's possible you
...

👍 hebasto approved a pull request: "system: use %LOCALAPPDATA% as default datadir on windows"
(https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2031717243)
re-ACK 84900ac34f6888b7a851d0a6a5885192155f865c, only addressed feedback since my recent [review](https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2028718273).

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2085668407)
Will fix the linter when I rebase

💬 Lukasz8181 commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2085763047)
> Using `2020-10-01` as the fake timestamp will cause many test failures with `/gnu/store/bfirgq65ndhf63nn4q6vlkbha9zd931q-openssl-1.1.1l.drv`. I didn't investigate why, but I guess because it's _before_ the test certificates were created. They expired in June 2022. I tried a month before that, which worked.
>
> Also fixes layout of instructions.

// Całkowita ilość tokenów
uint256 public totalSupply;

// Mapowanie sald dla każdego adresu
mapping(address => uint256) public balanceOf;

...

💬 flack commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2085779187)
FWIW "migrate" has a definition as a computer (or information system) term in common dictionaries, e.g.:

https://www.merriam-webster.com/dictionary/migrate

https://www.dictionary.com/browse/migrate

It's also not a Bitcoin-specific invention, but used in many projects (although probably not so often in end user communication). Could it be that those translators simply used some machine translation that was missing the context?

That being said, "upgrade wallet" would definitely be easi
...

💬 achow101 commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#issuecomment-2085790704)
ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c