π¬ toolsopen commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788341767)
https://github.com/bitcoin/bitcoin/issues/12115
<img width="921" alt="image" src="https://github.com/bitcoin/bitcoin/assets/21998212/eb9c1ef3-bb33-4600-a5d7-f9fd0dcddfbe">
@achow101 @maflcko @willcl-ark @gregwebs I saw that this issue was raised in 2018, but no attention was paid to it
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788341767)
https://github.com/bitcoin/bitcoin/issues/12115
<img width="921" alt="image" src="https://github.com/bitcoin/bitcoin/assets/21998212/eb9c1ef3-bb33-4600-a5d7-f9fd0dcddfbe">
@achow101 @maflcko @willcl-ark @gregwebs I saw that this issue was raised in 2018, but no attention was paid to it
π¬ ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378361925)
I think the usage of virtual size to build the ancestor score of a package might come with incentive suboptimal block template if you have a significant number of segwit spends constituting it (sounds 90% as of 2023). A 500 vbyte transaction A paying 100 sats with 100 vbytes of witnesses isnβt equivalent to a 500 vbyte transaction B paying 100 sats with 20 vbytes of witnesses considering block max size is checked against `MAX_BLOCK_WEIGHT` in `CheckBlock()`.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378361925)
I think the usage of virtual size to build the ancestor score of a package might come with incentive suboptimal block template if you have a significant number of segwit spends constituting it (sounds 90% as of 2023). A 500 vbyte transaction A paying 100 sats with 100 vbytes of witnesses isnβt equivalent to a 500 vbyte transaction B paying 100 sats with 20 vbytes of witnesses considering block max size is checked against `MAX_BLOCK_WEIGHT` in `CheckBlock()`.
π€ ariard reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1707520236)
Review at commit β[txpackages] add AncestorPackage for linearizing packagesβ.
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1707520236)
Review at commit β[txpackages] add AncestorPackage for linearizing packagesβ.
π¬ ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378371630)
If my understanding of the guarantees of the `PackageEntry` comparison operator is correct, a breakage of the guarantee would be a yield `AncestorPackage` that produce a consensus-invalid block template, e.g a more incentive-compatible subset of descendants where the parents are sorted after. This comment to understand how test coverage should be built for this PR.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378371630)
If my understanding of the guarantees of the `PackageEntry` comparison operator is correct, a breakage of the guarantee would be a yield `AncestorPackage` that produce a consensus-invalid block template, e.g a more incentive-compatible subset of descendants where the parents are sorted after. This comment to understand how test coverage should be built for this PR.
π¬ ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378372594)
I donβt know if it has been discussed before in the review of this PR, though in case of a tie sounds the breaker should not be on the absolute number of in-package ancestor though on their accumulated vsize ? E.g package entry A might have 1 ancestor of 1 kvB and package entry B might have 3 ancestors of 200 vbytes each.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378372594)
I donβt know if it has been discussed before in the review of this PR, though in case of a tie sounds the breaker should not be on the absolute number of in-package ancestor though on their accumulated vsize ? E.g package entry A might have 1 ancestor of 1 kvB and package entry B might have 3 ancestors of 200 vbytes each.
π¬ ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378363626)
I think one limitation of the partial set of ancestors being used to evaluate the ancestor-score of a package _could_ be the edge-case where unconfirmed ancestors are replaced in the mempool by better same-nonwitness-data-in-mempool candidates, with increased or diminished witness size. This is not an issue right now as we donβt allow wtxid-replacement in `PreChecks()`.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378363626)
I think one limitation of the partial set of ancestors being used to evaluate the ancestor-score of a package _could_ be the edge-case where unconfirmed ancestors are replaced in the mempool by better same-nonwitness-data-in-mempool candidates, with increased or diminished witness size. This is not an issue right now as we donβt allow wtxid-replacement in `PreChecks()`.
π¬ maflcko commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788505868)
Again, it is hard or impossible to fix your problem if it is wasn't properly diagnosed. The delay *could* be entirely unrelated to the wallet.
However, if the delay is caused by storage speed limitations in combination with the BDB flush behavior, a solution could be to migrate your wallet to sqlite. The (experimental) `migratewallet` RPC is only available in 25.x or the upcoming 26.x.
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788505868)
Again, it is hard or impossible to fix your problem if it is wasn't properly diagnosed. The delay *could* be entirely unrelated to the wallet.
However, if the delay is caused by storage speed limitations in combination with the BDB flush behavior, a solution could be to migrate your wallet to sqlite. The (experimental) `migratewallet` RPC is only available in 25.x or the upcoming 26.x.
π maflcko's pull request is ready for review: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735)
(https://github.com/bitcoin/bitcoin/pull/28735)
π¬ maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1788558389)
Taken out of draft. Future improvements to `chaincodelabs/libmultiprocess` can be done in the future.
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1788558389)
Taken out of draft. Future improvements to `chaincodelabs/libmultiprocess` can be done in the future.
π¬ naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1378500641)
636d927c4a9cd5cbf4d0c22e1c25dc44ecaf7a69
Is it still fair to call this state *initial* block download?
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1378500641)
636d927c4a9cd5cbf4d0c22e1c25dc44ecaf7a69
Is it still fair to call this state *initial* block download?
π naumenkogs opened a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765)
Keep track of per-peer reconciliation sets containing transactions to be exchanged efficiently. The remaining transactions are announced via usual flooding.
Erlay Project Tracking: #28646
(https://github.com/bitcoin/bitcoin/pull/28765)
Keep track of per-peer reconciliation sets containing transactions to be exchanged efficiently. The remaining transactions are announced via usual flooding.
Erlay Project Tracking: #28646
β
naumenkogs closed a pull request: "p2p: Fill reconciliation sets and request reconciliation (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/26283)
(https://github.com/bitcoin/bitcoin/pull/26283)
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#issuecomment-1788633580)
I decided to split this off into #28765.
(https://github.com/bitcoin/bitcoin/pull/26283#issuecomment-1788633580)
I decided to split this off into #28765.
π hebasto approved a pull request: "Fix crash on selecting "Mask values" in transaction view"
(https://github.com/bitcoin-core/gui/pull/774#pullrequestreview-1707876119)
ACK e26e665f9f64a962dd56053be817cc953e714847, tested on Ubuntu 22.04.
(https://github.com/bitcoin-core/gui/pull/774#pullrequestreview-1707876119)
ACK e26e665f9f64a962dd56053be817cc953e714847, tested on Ubuntu 22.04.
π¬ fanquake commented on pull request "Fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788686965)
Will backport this once merged.
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788686965)
Will backport this once merged.
π¬ hebasto commented on pull request "Fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788687291)
> https://github.com/bitcoin-core/gui/blob/3c0b66c2ece7695ab59f466d7b4d81a4c18ffd76/src/qt/transactionview.cpp#L532-L533
>
> The last one (#L533) was added by the #708 (merged in v25 - I wanted to say I'm innocent but no haha), so another way to solve this issue is just by removing #L532, which I've tested and works fine.
It will still allow to grow the size of `m_opened_dialogs` unbounded.
The current PR branch tackles with `m_opened_dialogs` size, which is optimum solution.
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788687291)
> https://github.com/bitcoin-core/gui/blob/3c0b66c2ece7695ab59f466d7b4d81a4c18ffd76/src/qt/transactionview.cpp#L532-L533
>
> The last one (#L533) was added by the #708 (merged in v25 - I wanted to say I'm innocent but no haha), so another way to solve this issue is just by removing #L532, which I've tested and works fine.
It will still allow to grow the size of `m_opened_dialogs` unbounded.
The current PR branch tackles with `m_opened_dialogs` size, which is optimum solution.
π hebasto merged a pull request: "Fix crash on selecting "Mask values" in transaction view"
(https://github.com/bitcoin-core/gui/pull/774)
(https://github.com/bitcoin-core/gui/pull/774)
π¬ 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.