💬 ajtowns commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1822050550)
> Yeah, my only (rather minor) reservation about `UNPARSEABLE<...>` is that I think of `<...>` as a way of indicating "stuff being pushed",

Yeah, I guess being able to scan and say `<...>` means data is going onto the stack makes sense.

> So maybe just `UNPARSEABLE(...)`?

Could do `UNPARSEABLE:04010203` or something (colon instead of brackets).

(Arguably, everything after the first OP_SUCCESS in a tapscript could also be considered trailing hex that shouldn't be decoded. Probably mo
...

💬 ajtowns commented on pull request "refactor: Make CTxMemPoolEntry only explicitly copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#discussion_r1401481010)
Might be good to add a comment explaining why copying this is often a bad thing and should only be done explicitly? (I think it's because `m_parents` and `m_children` need to stay in sync with the container holding this entry -- if you leave the copy out of the container, those sets will become bad; if you put it back into the container, those sets will probably be wrong)

🤔 ajtowns reviewed a pull request: "refactor: Make CTxMemPoolEntry only explicitly copyable"
(https://github.com/bitcoin/bitcoin/pull/28903#pullrequestreview-1743536103)
ACK 705e3f1de00bf30d728addd52a790a139d948e32

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1401611762)
Thanks, will do in one of the follow-up pulls, or here if I have to re-touch, whichever happens earlier.

📝 maflcko converted_to_draft a pull request: "refactor: VectorWriter and SpanReader without nVersion"
(https://github.com/bitcoin/bitcoin/pull/28912)
The serialize version is unused, so remove it. This also allows to remove `GCS_SER_VERSION` and allows a scripted-diff to remove most of `CDataStream`.

💬 hebasto commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#issuecomment-1822264312)
CI restarted.

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1401672632)
in 9426d3e7e5c884cd9ed31f2569a38a6e3712cc2c

We do other upgrade procedures in `LoadWallet()` function and only if `result != DBErrors::LOAD_OK`
Do we want to do the same for `UpgradeToGlobalHDKey`?

💬 Fei-Zodiac commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1822330742)
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKEORRRTE7C2NAKPR4BNNS3YFW2BHAVCNFSM6AAAAAATD3FIMWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBTHAYTCNRQGM>

S3RK ***@***.***> 于2023年11月22日周三 16:32写道：

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/wallet/walletdb.cpp
> <https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1401672632>:
>
> > @@ -1063,6 +1067,9 @@ static DBErrors LoadDescriptorWalletRecords(CWallet*
...

💬 n1bor commented on issue "v26.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1822425942)
Running fine on:
Intel(R) Atom(TM) CPU C2338 @ 1.74GHz - 2 Cores and 4Gb RAM
Ubuntu 20.04.6 LTS

With 453 connections:
Bitcoin Core client v26.0.0rc2 - server 70016/Satoshi:26.0.0/

ipv4 ipv6 onion total block
in 282 111 50 443
out 3 2 5 10 2
total 285 113 55 453

💬 Oeunchamreun90 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1401779127)
Hello

💬 ryanofsky commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1401780527)
Thanks! That approach of being able to combine different params is clever and solves my problem, avoiding boilerplate from having to create different streams for each object type.

Maybe at some point it would be useful to support default parameter values, too, so parameters like CNetAddr::V2 and CAddress::V2_NETWORK could be assumed if not specified. For some serialization parameters it may be better to be explicit, but for others there could be downsides. For example if I hardcode V2 in mult
...

💬 maflcko commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1401788091)
commit message: This is not a refactor

💬 maflcko commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1401790316)
Why the c-style cast at all?

📝 hebasto opened a pull request: "ci: Update apt cache"
(https://github.com/bitcoin/bitcoin/pull/28925)
This PR aims to fix the recent errors in the "test each commit" CI job.

💬 hebasto commented on pull request "ci: Update apt cache":
(https://github.com/bitcoin/bitcoin/pull/28925#issuecomment-1822461396)
cc @maflcko

💬 maflcko commented on pull request "ci: Update apt cache":
(https://github.com/bitcoin/bitcoin/pull/28925#issuecomment-1822489518)
lgtm ACK 710da28c72259e1054bb63d2d81a8f3aea4c9000

💬 ismaelsadeeq commented on pull request "ci: Update apt cache":
(https://github.com/bitcoin/bitcoin/pull/28925#issuecomment-1822496249)
utACK 710da28c72259e1054bb63d2d81a8f3aea4c9000

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1401835680)
Fixed Thanks

🚀 hebasto merged a pull request: "ci: Update apt cache"
(https://github.com/bitcoin/bitcoin/pull/28925)

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#issuecomment-1822539544)
Thanks for your review @TheCharlatan
Update from 969f5ec to 91504cb
[Compare 969f5ec to 91504cb](https://github.com/bitcoin/bitcoin/compare/969f5ec4a8f823e593bb71cf59c523c49774f069..91504cbe0de2b74ef1aa2709761aaf0597ec66a2)
* Using explicit constructors and list initialization for `NewMempoolTransactionInfo` and `RemovedMempoolTransactionInfo` struct
* Removed redundant `boolean` argument in `CBlockPolicyEstimator::removeTx`
* Removed unused argument names in the overridden interface `CVa
...

💬 fanquake commented on pull request "ci: Avoid toolset ambiguity that MSVC can't handle":
(https://github.com/bitcoin/bitcoin/pull/28905#issuecomment-1822571419)
Do we know why this broke again? I guess this is something we'll just have to maintain forever? That's a shame, because it also adds ~30% runtime to this CI job.