💬 jsarenik commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1794367265)
@ajtowns wrote:
> What's the actual problem you're trying to solve here? Would it work to have the signet miner only generate blocks when the mempool has entries, and sit idle when it's empty, for instance? If you're trying to generate many blocks, then regtest is already better as it doesn't require a baseline proof of work, and simply having multiple regtest configs probably makes more sense.
The last [comment](https://pricey.pages.dev/loglog) makes a point. It really is hard when anyone c
...
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1794367265)
@ajtowns wrote:
> What's the actual problem you're trying to solve here? Would it work to have the signet miner only generate blocks when the mempool has entries, and sit idle when it's empty, for instance? If you're trying to generate many blocks, then regtest is already better as it doesn't require a baseline proof of work, and simply having multiple regtest configs probably makes more sense.
The last [comment](https://pricey.pages.dev/loglog) makes a point. It really is hard when anyone c
...
💬 maflcko commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1794381260)
Ok, but then I think this is a problem for the qa-assets repo to solve. As I said, BDB is not enabled in the fuzz CI, so this shouldn't cause CI issues.
One way for qa-assets to solve this would be to exclude inputs that crash bdb.
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1794381260)
Ok, but then I think this is a problem for the qa-assets repo to solve. As I said, BDB is not enabled in the fuzz CI, so this shouldn't cause CI issues.
One way for qa-assets to solve this would be to exclude inputs that crash bdb.
💬 maflcko commented on issue "test: wallet_miniscript.py fails with thread sanitizer ":
(https://github.com/bitcoin/bitcoin/issues/28800#issuecomment-1794386419)
This also happens on the qemu-s390x and valgrind CI tasks
(https://github.com/bitcoin/bitcoin/issues/28800#issuecomment-1794386419)
This also happens on the qemu-s390x and valgrind CI tasks
💬 maflcko commented on pull request "build: Drop no longer needed MSVC warning suppressions":
(https://github.com/bitcoin/bitcoin/pull/28798#issuecomment-1794394106)
lgtm ACK 33223f9d550b66885a15e2fe45147d629cc359d2
(https://github.com/bitcoin/bitcoin/pull/28798#issuecomment-1794394106)
lgtm ACK 33223f9d550b66885a15e2fe45147d629cc359d2
✅ maflcko closed a pull request: "RPC/Wallet: Access wallets via interfaces::Wallet"
(https://github.com/bitcoin/bitcoin/pull/26082)
(https://github.com/bitcoin/bitcoin/pull/26082)
💬 maflcko commented on pull request "RPC/Wallet: Access wallets via interfaces::Wallet":
(https://github.com/bitcoin/bitcoin/pull/26082#issuecomment-1794410487)
Closing for now. Please leave a comment here, to have it re-opened by someone.
(https://github.com/bitcoin/bitcoin/pull/26082#issuecomment-1794410487)
Closing for now. Please leave a comment here, to have it re-opened by someone.
🤔 Sjors reviewed a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1714571006)
Concept ACK. Lightly reviewed the refactoring (as of a20c1d2b81a702f97267d4f098f13ed80e9320cf) which all looks reasonable, but I'm not very familiar with mempool code.
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1714571006)
Concept ACK. Lightly reviewed the refactoring (as of a20c1d2b81a702f97267d4f098f13ed80e9320cf) which all looks reasonable, but I'm not very familiar with mempool code.
💬 Sjors commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382967058)
ee79f05f1fef57bfbea24333a46e525385d02319: `{`
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382967058)
ee79f05f1fef57bfbea24333a46e525385d02319: `{`
💬 Sjors commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382997719)
6b6793e6835dd2ddf367490362042bf34eb260a3: The only caller `checkChainLimits` of is the wallet's `CreateTransactionInternal`, which will say "Transaction has too long of a mempool chain" regardless of which limit is hit.
A (wallet) followup should propagate `err_string` up.
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382997719)
6b6793e6835dd2ddf367490362042bf34eb260a3: The only caller `checkChainLimits` of is the wallet's `CreateTransactionInternal`, which will say "Transaction has too long of a mempool chain" regardless of which limit is hit.
A (wallet) followup should propagate `err_string` up.
💬 maflcko commented on pull request "Fix typos and suggest doing so before branch-off":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794414689)
Are you still working on this? Otherwise, I think it can be closed, so that it can be picked up by someone else, if it is still relevant.
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794414689)
Are you still working on this? Otherwise, I think it can be closed, so that it can be picked up by someone else, if it is still relevant.
💬 Sjors commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383019317)
ee79f05f1fef57bfbea24333a46e525385d02319: this fixes the dangling warning:
```cpp
const CTxMemPoolEntry& entry{Assert(m_pool.GetEntry(txid)).value()};
```
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383019317)
ee79f05f1fef57bfbea24333a46e525385d02319: this fixes the dangling warning:
```cpp
const CTxMemPoolEntry& entry{Assert(m_pool.GetEntry(txid)).value()};
```
💬 Sjors commented on pull request "Fix typos and suggest doing so before branch-off":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794431063)
I was still waiting for feedback from maintainers, but I'll just drop the last commit.
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794431063)
I was still waiting for feedback from maintainers, but I'll just drop the last commit.
💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383022517)
Thank, might be a good idea to bracket initialize in other places too.
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383022517)
Thank, might be a good idea to bracket initialize in other places too.
💬 willcl-ark commented on pull request "doc: Add offline signing tutorial":
(https://github.com/bitcoin/bitcoin/pull/28363#issuecomment-1794436797)
ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33
Looks good to me!
(https://github.com/bitcoin/bitcoin/pull/28363#issuecomment-1794436797)
ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33
Looks good to me!
💬 dergoegge commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1794443638)
Here's a coverage report: https://dergoegge.github.io/bitcoin-coverage/pr28578/fuzz.coverage/index.html (~1000h CPU time).
Is there a reason not to cover more of `DescriptorScriptPubKeyMan` in this PR? e.g. `DescriptorScriptPubKeyMan::SignTransaction` or `DescriptorScriptPubKeyMan::FillPSBT`.
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1794443638)
Here's a coverage report: https://dergoegge.github.io/bitcoin-coverage/pr28578/fuzz.coverage/index.html (~1000h CPU time).
Is there a reason not to cover more of `DescriptorScriptPubKeyMan` in this PR? e.g. `DescriptorScriptPubKeyMan::SignTransaction` or `DescriptorScriptPubKeyMan::FillPSBT`.
🚀 fanquake merged a pull request: "build: Drop no longer needed MSVC warning suppressions"
(https://github.com/bitcoin/bitcoin/pull/28798)
(https://github.com/bitcoin/bitcoin/pull/28798)
💬 fanquake commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794459605)
```bash
test/functional/feature_assumeutxo.py:78: refering ==> referring
test/functional/feature_assumeutxo.py:115: refering ==> referring
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794459605)
```bash
test/functional/feature_assumeutxo.py:78: refering ==> referring
test/functional/feature_assumeutxo.py:115: refering ==> referring
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
👍 dergoegge approved a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789#pullrequestreview-1714698328)
ACK fa7ba926300f44b350d04262e569cc607e4991d8
(https://github.com/bitcoin/bitcoin/pull/28789#pullrequestreview-1714698328)
ACK fa7ba926300f44b350d04262e569cc607e4991d8
💬 maflcko commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794494686)
> I was still waiting for feedback from maintainers
DrahtBot has a summary comment, which has links to all feedback, including that of maintainers, see https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1750139214
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794494686)
> I was still waiting for feedback from maintainers
DrahtBot has a summary comment, which has links to all feedback, including that of maintainers, see https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1750139214
💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1794513714)
Updated a20c1d2b81a702f97267d4f098f13ed80e9320cf -> 836a0a7566e166556f80303a9b3dd9560587159c ([simplifyMemPoolInteractions_7](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_7) -> [simplifyMemPoolInteractions_8](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_7..simplifyMemPoolInteractions_8))
* Addressed @Sjors's [comment](https://github.com/bitcoin/bit
...
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1794513714)
Updated a20c1d2b81a702f97267d4f098f13ed80e9320cf -> 836a0a7566e166556f80303a9b3dd9560587159c ([simplifyMemPoolInteractions_7](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_7) -> [simplifyMemPoolInteractions_8](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_7..simplifyMemPoolInteractions_8))
* Addressed @Sjors's [comment](https://github.com/bitcoin/bit
...