💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690313716)
Attempted cleaning that up, but didn't uncover anything useful to the end users, unlike #30444, so probably not worth following up on:
```diff
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index 89c03c1647..dab8b81128 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -274,8 +274,8 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu
// extract and validate vout
const std::string& strVout = vStrInputParts[1];
- int64_t vou
...
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690313716)
Attempted cleaning that up, but didn't uncover anything useful to the end users, unlike #30444, so probably not worth following up on:
```diff
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index 89c03c1647..dab8b81128 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -274,8 +274,8 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu
// extract and validate vout
const std::string& strVout = vStrInputParts[1];
- int64_t vou
...
💬 mzumsande commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690338607)
nit: After this, the log
`Committed X changed transaction outputs (out of Y) to coin database...`
has become less interesting because we only iterate over dirty coins, so `X=Y` always.
I might be more helpful to report the initial size of `m_map` of the cursor (before entries are deleted from it) as the total.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690338607)
nit: After this, the log
`Committed X changed transaction outputs (out of Y) to coin database...`
has become less interesting because we only iterate over dirty coins, so `X=Y` always.
I might be more helpful to report the initial size of `m_map` of the cursor (before entries are deleted from it) as the total.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690361434)
There are other places that could be cleaned up, such as obviated dirty checks like [here](https://github.com/bitcoin/bitcoin/pull/28280/files#diff-cafbe1353eff6084b73fd3b6c3dee603e0827348fdd2fe12dfad1e01003a84edR118) and [here](https://github.com/bitcoin/bitcoin/pull/28280/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77cR195), but this should probably done after a https://github.com/bitcoin/bitcoin/pull/18746 revival so there's no confusion.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690361434)
There are other places that could be cleaned up, such as obviated dirty checks like [here](https://github.com/bitcoin/bitcoin/pull/28280/files#diff-cafbe1353eff6084b73fd3b6c3dee603e0827348fdd2fe12dfad1e01003a84edR118) and [here](https://github.com/bitcoin/bitcoin/pull/28280/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77cR195), but this should probably done after a https://github.com/bitcoin/bitcoin/pull/18746 revival so there's no confusion.
💬 mzumsande commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690365906)
yes, no need to do that here. I just mentioned the logging here because I was interested in the ratios to assess the efficiency of this PR, and had to write my own log.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690365906)
yes, no need to do that here. I just mentioned the logging here because I was interested in the ratios to assess the efficiency of this PR, and had to write my own log.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690367507)
Also, that line predates #17487, back when all entries were dirty anyways.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690367507)
Also, that line predates #17487, back when all entries were dirty anyways.
👍 TheCharlatan approved a pull request: "guix: GCC 12 consolidation"
(https://github.com/bitcoin/bitcoin/pull/30511#pullrequestreview-2197680394)
ACK d1592d2eee1913e734a4f92907e796eb3350c64a
Guix build (aarch64):
```
4fba0aba8be59fd967b099e19faf6ba6770a83de1e7bee745c16f91fa3eb92fe guix-build-d1592d2eee19/output/aarch64-linux-gnu/SHA256SUMS.part
37e93c9235db88f58e5a45e8a54e1d57602f9107cd23676c91252509b2fe3bb5 guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu-debug.tar.gz
184fb61be0843b069c4c0d92119b9e29e895f11ef8687b220ab3898999c06afa guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d
...
(https://github.com/bitcoin/bitcoin/pull/30511#pullrequestreview-2197680394)
ACK d1592d2eee1913e734a4f92907e796eb3350c64a
Guix build (aarch64):
```
4fba0aba8be59fd967b099e19faf6ba6770a83de1e7bee745c16f91fa3eb92fe guix-build-d1592d2eee19/output/aarch64-linux-gnu/SHA256SUMS.part
37e93c9235db88f58e5a45e8a54e1d57602f9107cd23676c91252509b2fe3bb5 guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu-debug.tar.gz
184fb61be0843b069c4c0d92119b9e29e895f11ef8687b220ab3898999c06afa guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d
...
💬 TheCharlatan commented on pull request "guix: GCC 12 consolidation":
(https://github.com/bitcoin/bitcoin/pull/30511#discussion_r1690370855)
Hope this gets merged soon so we can drop this again.
(https://github.com/bitcoin/bitcoin/pull/30511#discussion_r1690370855)
Hope this gets merged soon so we can drop this again.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385057)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385057)
Done.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385537)
Added the mermaid code as comments to the benchmark.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385537)
Added the mermaid code as comments to the benchmark.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385632)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385632)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385744)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385744)
Done.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386555)
I've added comments explaining this, and also moved the "test correspondence between Cluster and DepGraph" to a new `VerifyDepGraphFromCluster` function that's invoked in both the unit and fuzz tests.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386555)
I've added comments explaining this, and also moved the "test correspondence between Cluster and DepGraph" to a new `VerifyDepGraphFromCluster` function that's invoked in both the unit and fuzz tests.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386667)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386667)
Done.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2248844497)
added fuzz and unit tests which cover more than the functional tests can alone
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2248844497)
added fuzz and unit tests which cover more than the functional tests can alone
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386739)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386739)
Done.
💬 instagibbs commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2248854763)
reACK https://github.com/bitcoin/bitcoin/pull/30275/commits/25bf86a225b0df3f48ade1016b47f5ee1636b988
tests make me feel much better about this thanks
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2248854763)
reACK https://github.com/bitcoin/bitcoin/pull/30275/commits/25bf86a225b0df3f48ade1016b47f5ee1636b988
tests make me feel much better about this thanks
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2248939831)
Was able to simplify this by dropping CustomBuildMessage/CustomReadMessage functions and tests. It now requires the latest version of the libmultiprocess library though.
Rebased 79cfc0d198faecec598f923e6bf83ccf8dc09bbb -> 6cccb436557c58e0ccd21ffe0eaf31058f5cb799 ([`pr/mine-types.2`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.2) -> [`pr/mine-types.3`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-ty
...
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2248939831)
Was able to simplify this by dropping CustomBuildMessage/CustomReadMessage functions and tests. It now requires the latest version of the libmultiprocess library though.
Rebased 79cfc0d198faecec598f923e6bf83ccf8dc09bbb -> 6cccb436557c58e0ccd21ffe0eaf31058f5cb799 ([`pr/mine-types.2`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.2) -> [`pr/mine-types.3`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-ty
...
💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2248990387)
Rebased fba04d7756c77ed9371c5dc90d3bebc9aa767f4b -> ec5f45b9186184cac9c64ed08b8b8cc596e301a3 ([`pr/mine.7`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.7) -> [`pr/mine.8`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.7-rebase..pr/mine.8)), moving most code that was here into base PRs #30509 and #30510. I also followed up various review suggestions above like fixing the top-level make target and dropping unneeded
...
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2248990387)
Rebased fba04d7756c77ed9371c5dc90d3bebc9aa767f4b -> ec5f45b9186184cac9c64ed08b8b8cc596e301a3 ([`pr/mine.7`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.7) -> [`pr/mine.8`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.7-rebase..pr/mine.8)), moving most code that was here into base PRs #30509 and #30510. I also followed up various review suggestions above like fixing the top-level make target and dropping unneeded
...
👍 theStack approved a pull request: "policy: Add PayToAnchor(P2A), `OP_1 <0x4e73>` as a standard output script for spending"
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2197876025)
Code-review ACK 67c3537f75bcf085bb98e3830649e93da124cb06
Left a non-blocking test nit. In the future a dedicated MiniWallet-mode for P2A could probably make sense (should be trivial to implement).
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2197876025)
Code-review ACK 67c3537f75bcf085bb98e3830649e93da124cb06
Left a non-blocking test nit. In the future a dedicated MiniWallet-mode for P2A could probably make sense (should be trivial to implement).
💬 theStack commented on pull request "policy: Add PayToAnchor(P2A), `OP_1 <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1690530356)
obviously way out of scope for this PR, but I wonder: could there be scenarios where it's useful/needed to have "signing support" for P2A outputs, by doing nothing and just succeed here? (probably not, as `createrawtransaction` with the P2A input is already sufficient to spend it).
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1690530356)
obviously way out of scope for this PR, but I wonder: could there be scenarios where it's useful/needed to have "signing support" for P2A outputs, by doing nothing and just succeed here? (probably not, as `createrawtransaction` with the P2A input is already sufficient to spend it).
💬 theStack commented on pull request "policy: Add PayToAnchor(P2A), `OP_1 <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1690520020)
nit, could alternatively use MiniWallet's `.send_to` method for less code (assuming that specifying nSequence explicitly is not important), e.g.:
```diff
diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
index cf3185271c..4b9ff3d138 100755
--- a/test/functional/mempool_accept.py
+++ b/test/functional/mempool_accept.py
@@ -391,16 +391,14 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
)
self.log.info('OP_1 <0x4e73> is able to b
...
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1690520020)
nit, could alternatively use MiniWallet's `.send_to` method for less code (assuming that specifying nSequence explicitly is not important), e.g.:
```diff
diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
index cf3185271c..4b9ff3d138 100755
--- a/test/functional/mempool_accept.py
+++ b/test/functional/mempool_accept.py
@@ -391,16 +391,14 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
)
self.log.info('OP_1 <0x4e73> is able to b
...