💬 zzzi2p commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2737607630)
Tunnels get built when the SAM session is opened, and are automatically rebuilt when they expire, the router does that for you as long as the SAM session stays open. A session is just a pool of inbound and outbound tunnels. I assume that's what you are trying to do here, have the session open and ready before it's needed.
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2737607630)
Tunnels get built when the SAM session is opened, and are automatically rebuilt when they expire, the router does that for you as long as the SAM session stays open. A session is just a pool of inbound and outbound tunnels. I assume that's what you are trying to do here, have the session open and ready before it's needed.
💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2737612354)
> Couldn't much of this be done by using a `just` file? I saw a PR a while that tried to add something about a `justfile` which is a way to wrap command to make them easier and more discoverable.
Yes it could also be done as a shell script or python script. If we could count on a justfile interpreter, or a portable shell, or python being installed on all platforms which bitcoin runs on, the changes in the two commits here a47cf680619cae5892d72355f56f5c5051fd5434 and cbc0bdd73ed0fd672307cf0f80
...
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2737612354)
> Couldn't much of this be done by using a `just` file? I saw a PR a while that tried to add something about a `justfile` which is a way to wrap command to make them easier and more discoverable.
Yes it could also be done as a shell script or python script. If we could count on a justfile interpreter, or a portable shell, or python being installed on all platforms which bitcoin runs on, the changes in the two commits here a47cf680619cae5892d72355f56f5c5051fd5434 and cbc0bdd73ed0fd672307cf0f80
...
💬 darosior commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2737618918)
Thanks for testing!
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2737618918)
Thanks for testing!
💬 yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2737655586)
> This is not accurate. You could have two UTXOs of different output types that have the same effective value but differing fees. Counterexample:
Yes, I redacted that comment later in the thread as my understand of the problem improved. As I posted last, there's even a test [here](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L427) that tests two UTXOs with the same effective_value yet different fees.
> However, since [CoinGrinder got merged](https://gi
...
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2737655586)
> This is not accurate. You could have two UTXOs of different output types that have the same effective value but differing fees. Counterexample:
Yes, I redacted that comment later in the thread as my understand of the problem improved. As I posted last, there's even a test [here](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L427) that tests two UTXOs with the same effective_value yet different fees.
> However, since [CoinGrinder got merged](https://gi
...
💬 yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2737675105)
However, doesn't the waste score account for long_term_fee_rate as well as fee_rate? In other-words, just comparing `fee` amounts and not `long_term_fee_rate` would not be enough to know if comparing two UTXOs have different was scores..
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2737675105)
However, doesn't the waste score account for long_term_fee_rate as well as fee_rate? In other-words, just comparing `fee` amounts and not `long_term_fee_rate` would not be enough to know if comparing two UTXOs have different was scores..
⚠️ luisschwab opened an issue: "Incorrect balance when dealing with coinbase UTXOs that will be mature on _h_ + 1"
(https://github.com/bitcoin/bitcoin/issues/32098)
Bitcoin Core is reporting the wrong balance when dealing with coinbase UTXOs that will become mature on the next block.
Scenario:
On regtest, I mine 101 blocks. Since coinbase maturity height is 100 blocks, I have 1 mature UTXO (50 BTC) at height 101. However, at height 102, another coinbase UTXO (50 BTC) will have matured and will be accepted by the mempool as an input to a transaction. Therefore, my _**spendable**_ balance at height 102 will be 50+50 BTC (2 UTXOs available for selection).
Cu
...
(https://github.com/bitcoin/bitcoin/issues/32098)
Bitcoin Core is reporting the wrong balance when dealing with coinbase UTXOs that will become mature on the next block.
Scenario:
On regtest, I mine 101 blocks. Since coinbase maturity height is 100 blocks, I have 1 mature UTXO (50 BTC) at height 101. However, at height 102, another coinbase UTXO (50 BTC) will have matured and will be accepted by the mempool as an input to a transaction. Therefore, my _**spendable**_ balance at height 102 will be 50+50 BTC (2 UTXOs available for selection).
Cu
...
💬 luisschwab commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2737775525)
Apparently this is a simple fix:
```diff
scr/wallet/wallet.cpp
int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const
{
AssertLockHeld(cs_wallet);
if (!wtx.IsCoinBase()) {
return 0;
}
int chain_depth = GetTxDepthInMainChain(wtx);
assert(chain_depth >= 0); // coinbase tx should not be conflicted
- return std::max(0, (COINBASE_MATURITY+1) - chain_depth);
+ return std::max(0, COINBASE_MATURITY - chain_depth);
}
```
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2737775525)
Apparently this is a simple fix:
```diff
scr/wallet/wallet.cpp
int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const
{
AssertLockHeld(cs_wallet);
if (!wtx.IsCoinBase()) {
return 0;
}
int chain_depth = GetTxDepthInMainChain(wtx);
assert(chain_depth >= 0); // coinbase tx should not be conflicted
- return std::max(0, (COINBASE_MATURITY+1) - chain_depth);
+ return std::max(0, COINBASE_MATURITY - chain_depth);
}
```
💬 maflcko commented on pull request "test: Fix intermittent issue in p2p_orphan_handling.py":
(https://github.com/bitcoin/bitcoin/pull/32092#discussion_r2004125240)
thx, done
(https://github.com/bitcoin/bitcoin/pull/32092#discussion_r2004125240)
thx, done
💬 maflcko commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2737798747)
I presume this was last touched in commit bf3a20a6e8cafdf723ef101af078df303ea06fec, which explains that there may be a race condition where the transaction is rejected, because the 100th block has not been relayed yet to the peer when the transaction is sent.
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2737798747)
I presume this was last touched in commit bf3a20a6e8cafdf723ef101af078df303ea06fec, which explains that there may be a race condition where the transaction is rejected, because the 100th block has not been relayed yet to the peer when the transaction is sent.
💬 maflcko commented on issue "Failure to run Fuzz tests when running with corpus":
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2737863589)
> libc++abi: terminating due to uncaught exception of type std::__1::ios_base::failure: DataStream::read(): end of data: unspecified iostream_category error
This looks like an upstream packaging bug or asan bug on your platform, given that it passes fine when asan is disabled on your platform.
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2737863589)
> libc++abi: terminating due to uncaught exception of type std::__1::ios_base::failure: DataStream::read(): end of data: unspecified iostream_category error
This looks like an upstream packaging bug or asan bug on your platform, given that it passes fine when asan is disabled on your platform.
💬 hebasto commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737914378)
FWIW, at the end of configuration, Qt 6 prints:
```
If reconfiguration fails for some reason, try removing 'CMakeCache.txt' from the build directory
Alternatively, you can add the --fresh flag to your CMake flags.
```
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737914378)
FWIW, at the end of configuration, Qt 6 prints:
```
If reconfiguration fails for some reason, try removing 'CMakeCache.txt' from the build directory
Alternatively, you can add the --fresh flag to your CMake flags.
```
🤔 maflcko reviewed a pull request: "contrib: Make deterministic-coverage error messages more readable"
(https://github.com/bitcoin/bitcoin/pull/32074#pullrequestreview-2700009626)
rebased with a move-only change, and replied to all feedback
(https://github.com/bitcoin/bitcoin/pull/32074#pullrequestreview-2700009626)
rebased with a move-only change, and replied to all feedback
💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004192240)
I found `Err(...)?` nicer as an early-return statement, but no strong opinion
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004192240)
I found `Err(...)?` nicer as an early-return statement, but no strong opinion
💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004194037)
In Rust shadowing is a feature, but happy to change, no strong opinion.
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004194037)
In Rust shadowing is a feature, but happy to change, no strong opinion.
💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004190137)
thx, done
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004190137)
thx, done
💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004196829)
Nice. I think this is sufficient as an approach for a dev-only tool. Happy to push it here, or review your follow-up. Just let me know.
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004196829)
Nice. I think this is sufficient as an approach for a dev-only tool. Happy to push it here, or review your follow-up. Just let me know.
💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004186423)
Yeah, the string ends up in `Err` eventually, so it should be type-safe enough for nothing to go wrong. At least I can't see any risk here.
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004186423)
Yeah, the string ends up in `Err` eventually, so it should be type-safe enough for nothing to go wrong. At least I can't see any risk here.
👋 maflcko's pull request is ready for review: "contrib: Make deterministic-coverage error messages more readable"
(https://github.com/bitcoin/bitcoin/pull/32074)
(https://github.com/bitcoin/bitcoin/pull/32074)
👍 hodlinator approved a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2700044703)
re-ACK 25b56fd9b469f8e5d36f0132c3b79a5214e3372a
Changes since [previous ACK](https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2661348106):
- Added commit switching to Bash by default which lets us avoid annoying "&& `" in later commit which was required by PowerShell. Confirmed CI still seems to be executing unit/functional/fuzz tests.
- Added comment explaining why we can't use ctest.
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2700044703)
re-ACK 25b56fd9b469f8e5d36f0132c3b79a5214e3372a
Changes since [previous ACK](https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2661348106):
- Added commit switching to Bash by default which lets us avoid annoying "&& `" in later commit which was required by PowerShell. Confirmed CI still seems to be executing unit/functional/fuzz tests.
- Added comment explaining why we can't use ctest.
💬 maflcko commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737988575)
> If reconfiguration fails for some reason, try removing 'CMakeCache.txt' from the build directory
> Alternatively, you can add the --fresh flag to your CMake flags.
I don't think I've seen issues where re-configuration fails. So far the linked issues here were about linker errors.
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737988575)
> If reconfiguration fails for some reason, try removing 'CMakeCache.txt' from the build directory
> Alternatively, you can add the --fresh flag to your CMake flags.
I don't think I've seen issues where re-configuration fails. So far the linked issues here were about linker errors.