💬 fanquake commented on pull request "Fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788699281)
Added to https://github.com/bitcoin/bitcoin/pull/28754 for 26.x.
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788699281)
Added to https://github.com/bitcoin/bitcoin/pull/28754 for 26.x.
💬 dergoegge commented on pull request "Fuzz: Check individual and package transaction invariants":
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1378603808)
```suggestion
std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns, const PackageMempoolAcceptResult& result, bool expect_valid,
const CTxMemPool* mempool);
```
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1378603808)
```suggestion
std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns, const PackageMempoolAcceptResult& result, bool expect_valid,
const CTxMemPool* mempool);
```
👍 hebasto approved a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707905519)
re-ACK e4e84790f62990f31a519f1ec0e8cc16e93a3c3b, only a backport of https://github.com/bitcoin-core/gui/pull/774 added.
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707905519)
re-ACK e4e84790f62990f31a519f1ec0e8cc16e93a3c3b, only a backport of https://github.com/bitcoin-core/gui/pull/774 added.
🚀 fanquake merged a pull request: "build: remove duplicate `-lminiupnpc` linking"
(https://github.com/bitcoin/bitcoin/pull/28755)
(https://github.com/bitcoin/bitcoin/pull/28755)
👍 TheCharlatan approved a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707914636)
Re-ACK e4e84790f62990f31a519f1ec0e8cc16e93a3c3b
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707914636)
Re-ACK e4e84790f62990f31a519f1ec0e8cc16e93a3c3b
💬 josibake commented on pull request "[26.x] Backports for rc2":
(https://github.com/bitcoin/bitcoin/pull/28754#issuecomment-1788725154)
ACK https://github.com/bitcoin/bitcoin/commit/e4e84790f62990f31a519f1ec0e8cc16e93a3c3b
(https://github.com/bitcoin/bitcoin/pull/28754#issuecomment-1788725154)
ACK https://github.com/bitcoin/bitcoin/commit/e4e84790f62990f31a519f1ec0e8cc16e93a3c3b
🚀 fanquake merged a pull request: "addrman: log AS only when using asmap"
(https://github.com/bitcoin/bitcoin/pull/28729)
(https://github.com/bitcoin/bitcoin/pull/28729)
💬 glozow commented on pull request "rpc: distinguish between vsize and sigop-adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1378636536)
IIUC the suggestion is:
- Add docs for where vsize is sigop-adjusted and where it isn't.
- Add some doc/policy/?.md describing feerate, vsize, mining score
- Add an optional "sigopsize" result for `getrawtransaction` and the mempool RPCs when we're able to provide it
Will work on this
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1378636536)
IIUC the suggestion is:
- Add docs for where vsize is sigop-adjusted and where it isn't.
- Add some doc/policy/?.md describing feerate, vsize, mining score
- Add an optional "sigopsize" result for `getrawtransaction` and the mempool RPCs when we're able to provide it
Will work on this
🚀 fanquake merged a pull request: "test: make python p2p not send getaddr on incoming connections"
(https://github.com/bitcoin/bitcoin/pull/28632)
(https://github.com/bitcoin/bitcoin/pull/28632)
👋 fanquake's pull request is ready for review: "build: Windows SSP roundup"
(https://github.com/bitcoin/bitcoin/pull/28461)
(https://github.com/bitcoin/bitcoin/pull/28461)
🚀 fanquake merged a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754)
(https://github.com/bitcoin/bitcoin/pull/28754)
💬 fanquake commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1788752667)
cc @TheCharlatan
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1788752667)
cc @TheCharlatan
💬 dergoegge commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378636413)
nit: the naming is slightly inconsistent now, it would be nicer if all the function are either `IsPackage*` or `Is*Package` instead of a mix. Would also not be opposed to turning `Package` into a class and have these be member functions.
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378636413)
nit: the naming is slightly inconsistent now, it would be nicer if all the function are either `IsPackage*` or `Is*Package` instead of a mix. Would also not be opposed to turning `Package` into a class and have these be member functions.
💬 dergoegge commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378645903)
```suggestion
CMutableTransaction dup_tx;
dup_tx.vin.emplace_back(uint256{}, 0);
dup_tx.vin.emplace_back(uint256{}, 0);
Package package_with_dup_tx{MakeTransactionRef(dup_tx)};
BOOST_CHECK(IsConsistentPackage(package_with_dup_tx));
package_with_dup_tx.emplace_back(create_placeholder_tx(1, 1));
BOOST_CHECK(IsConsistentPackage(package_with_dup_tx));
```
`dup_tx` is otherwise unused and does not conflict with itself.
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378645903)
```suggestion
CMutableTransaction dup_tx;
dup_tx.vin.emplace_back(uint256{}, 0);
dup_tx.vin.emplace_back(uint256{}, 0);
Package package_with_dup_tx{MakeTransactionRef(dup_tx)};
BOOST_CHECK(IsConsistentPackage(package_with_dup_tx));
package_with_dup_tx.emplace_back(create_placeholder_tx(1, 1));
BOOST_CHECK(IsConsistentPackage(package_with_dup_tx));
```
`dup_tx` is otherwise unused and does not conflict with itself.
💬 dergoegge commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378634066)
This can be dropped from the header, nothing is using it externally.
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378634066)
This can be dropped from the header, nothing is using it externally.
💬 fanquake commented on issue "Release schedule for 26.0":
(https://github.com/bitcoin/bitcoin/issues/27758#issuecomment-1788759649)
v26.0rc1 was basically dead on arrival due to an issue with the macos codesigned binary. This has been fixed, and a number of other bugfixes were also backported. [v26.0rc2](https://github.com/bitcoin/bitcoin/releases/tag/v26.0rc2) has now been tagged.
(https://github.com/bitcoin/bitcoin/issues/27758#issuecomment-1788759649)
v26.0rc1 was basically dead on arrival due to an issue with the macos codesigned binary. This has been fixed, and a number of other bugfixes were also backported. [v26.0rc2](https://github.com/bitcoin/bitcoin/releases/tag/v26.0rc2) has now been tagged.
📝 dergoegge opened a pull request: "Improve peformance of CTransaction::HasWitness (28107 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/28766)
This addresses https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1364961590 from #28107.
(https://github.com/bitcoin/bitcoin/pull/28766)
This addresses https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1364961590 from #28107.
👍 maflcko approved a pull request: "Use serialization parameters for CTransaction"
(https://github.com/bitcoin/bitcoin/pull/28438#pullrequestreview-1707872957)
lgtm, left some style questions.
lgtm ACK e5cb04009095c21517be71c0bf21cde4ed0490a7 📿
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trus
...
(https://github.com/bitcoin/bitcoin/pull/28438#pullrequestreview-1707872957)
lgtm, left some style questions.
lgtm ACK e5cb04009095c21517be71c0bf21cde4ed0490a7 📿
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trus
...
💬 maflcko commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1378613214)
This compiles, so I guess it is fine. Two notes:
* This forces bip152-v2-only in the serialize code. I guess it is fine, given https://github.com/bitcoin/bitcoin/pull/20799.
* question: If `TransactionCompression` was ever implemented it wouldn't support serialization without witness? So wouldn't it be cleaner to just implement the `TransactionCompression` formatter as one that calls `(Un)SerializeTransaction` with the witness params set?
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1378613214)
This compiles, so I guess it is fine. Two notes:
* This forces bip152-v2-only in the serialize code. I guess it is fine, given https://github.com/bitcoin/bitcoin/pull/20799.
* question: If `TransactionCompression` was ever implemented it wouldn't support serialization without witness? So wouldn't it be cleaner to just implement the `TransactionCompression` formatter as one that calls `(Un)SerializeTransaction` with the witness params set?
💬 maflcko commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1378584333)
review note: I presume reviewers are supposed to set `--function-context` and verify each time that `SERIALIZE_TRANSACTION_NO_WITNESS` was never embedded into the `CDataStream` in each context.
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1378584333)
review note: I presume reviewers are supposed to set `--function-context` and verify each time that `SERIALIZE_TRANSACTION_NO_WITNESS` was never embedded into the `CDataStream` in each context.