💬 Fi3 commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2225283367)
> I would then make another call to the interface to get those transactions, but by that that time the node has already forgotten the template it gave me and the mempool may have dropped some of the transactions it included.

Can you cache the tx data and keep them for the time that the template is valid. If you have more templates you save shared txs just one time.

📝 ryanofsky opened a pull request: "multiprocess: add bitcoin-mine test program "
(https://github.com/bitcoin/bitcoin/pull/30437)
Add [`bitcoin-mine`](https://github.com/ryanofsky/bitcoin/blob/pr/mine/src/bitcoin-mine.cpp#L19-L42) executable to test connecting to the `bitcoin-node` process over a unix socket and calling [`interface::Mining`](https://github.com/ryanofsky/bitcoin/blob/pr/mine/src/interfaces/mining.h) methods.

This could be useful as discussed in https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2222418506 to work on getting mining code working in an external process.

This PR is a draft, becaus
...

🤔 marcofleon reviewed a pull request: "fuzz: wallet: add target for `CreateTransaction`"
(https://github.com/bitcoin/bitcoin/pull/29936#pullrequestreview-2174428032)
Nice, coverage looks good now. The issue I'm seeing is the harness has really low stability. It's at 28% with the seed corpus I'm using. The only obvious source of randomness I'm seeing is in `CreateTransactionInternal`:
https://github.com/bitcoin/bitcoin/blob/4d6af61d879914a660e73db5c2f2e6c4d0aa8243/src/wallet/spend.cpp#L1164-L1166

Sometimes `changepos` isn't set so it does hit that line. Maybe there are other reasons for the low stability somewhere else?

💬 hebasto commented on pull request "depends: bump boost to 1.85.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2225302383)
> The CI failure https://cirrus-ci.com/task/4950868422819840:

Maybe revert 3bee51427a05075150721f0a05ead8f92e1ba019?

FWIW, in the CMake staging branch, `set(CMAKE_PLATFORM_HAS_INSTALLNAME FALSE)` is used in between the `project()` and `enable_language(CXX)` calls to [workaround](https://github.com/hebasto/bitcoin/blob/292e7da439a732092e3823fadd2379bbb7d3ff2b/CMakeLists.txt#L45-L53) it:
```cmake
#=============================
# Language setup
#=============================
if(CMAKE_SYS
...

🤔 maflcko reviewed a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2174408053)
left some comments

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675652818)
nit in a4ead4e51c04c75c7837fb53116062e79f502bbd (commit message): Could link directly to the discussion https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378 ?

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675665249)
The compiler will generate those, no? So seems fine to remove?

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675658574)
Both sides are trimmed. My recommendation would be to drop the first commit and instead add a unit test:

```diff
diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp
index b7892a2139..5602b33e17 100644
--- a/src/test/uint256_tests.cpp
+++ b/src/test/uint256_tests.cpp
@@ -102,6 +102,8 @@ BOOST_AUTO_TEST_CASE( basics ) // constructors, equality, inequality
BOOST_CHECK(uint256S("0x"+MaxL.ToString()) == MaxL);
BOOST_CHECK(uint256S(R1L.ToString()) == R1L);
BO
...

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675665868)
Same (the compiler will generate those)?

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675666153)
Also, the same for `inline uint160 uint160S(`.

💬 fanquake commented on pull request "depends: bump boost to 1.85.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#discussion_r1675668215)
Are we going to upstream this? Suprised there's not already an option to do this for at least `date_time`, given it's a stub library that only exists for back compat. Wonder if we can (easily) patch it out in the interim.

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675668347)
Also, could add a unit test that truncated input is parsed? (Sad as well, but again existing behavior, so probably good to keep for now)

💬 fanquake commented on pull request "depends: bump boost to 1.85.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2225307394)
> Maybe revert https://github.com/bitcoin/bitcoin/commit/3bee51427a05075150721f0a05ead8f92e1ba019?

A better solution would be to fix the CMake build systems, so we don't have to pointlessly compile stub libraries, that we ultimately don't even use.

💬 maflcko commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2225329321)
> The only obvious source of randomness I'm seeing is in `CreateTransactionInternal`:

Does it improve if you call `SeedRandomForTest` before every execution of every fuzz input?

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675687795)
(same commit): Also, you'd have to fix `WtxidFromString(` in the same commit?

💬 ryanofsky commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2225337100)
re: https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2225041208

> I don't think this can be solved with making both calls at the same time; the second call needs to reference a specific template that the first call generated.

A natural way to do this would be to have `createNewBlock` return an interface instead of a struct:

```c++
class Mining
{
virtual std::unique_ptr<MiningBlock> createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0;
}

class
...

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2225345559)
> , so I think it needs to be something inside the docker container. Also, I am not aware of a process inside the CI env that would echo back what it got.

Also interesting that no one reproduced this locally yet

✅ willcl-ark closed a pull request: "policy: Enable full-rbf by default"
(https://github.com/bitcoin/bitcoin/pull/28132)

💬 willcl-ark commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2225369634)
Thank everyone for your participation in this discussion and review of this pull request so far.

However the comments section here has become difficult to follow due to numerous off-topic comments, a few personal disagreements, and repetition of arguments.

In the interest of having a more productive and focused technical and philosophical discussion we are going to close and lock this PR.

-----

@petertodd: We strongly encourage you to open a new pull request for this proposal, as the
...

📝 fanquake locked a pull request: "policy: Enable full-rbf by default"
(https://github.com/bitcoin/bitcoin/pull/28132)
The following pools are have enabled full-rbf:

- [Foundry USA, 30%](https://mempool.space/tx/ca2ed3ea924131d4051569c9c59dc62191020674ba9b89606c3a725eefbc955b)
- [Antpool, 28%](https://mempool.space/tx/53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122ac6b1b7643)
- [ViaBTC, 14%](https://mempool.space/tx/4381156979261d6d46af74e0bc0553d24787bad9b3f2301fa9c9ba7c56e27bd3)
- [F2Pool, 10%](https://mempool.space/tx/e1213b60f1ac93b412a992deb0a79eebbde0032179c545350cc2136d0a3754fb)
- [Binance P
...