π¬ instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172597431)
older comment
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172597431)
older comment
π¬ furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2172705028)
Hmm, sorry for the very very late response @andrewtoth. I missed this message completely.
> I still think we would want to abandon the work queue if we are interrupted instead of waiting for it to finish, no? My naive approach of not waiting for the queue to be empty does not work though.
Yeah, I donβt think thatβs safe. Other threads might be waiting on the tasks' futures to complete, so exiting without notifying them would leave them blocked forever.
What we could do (and I think you me
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2172705028)
Hmm, sorry for the very very late response @andrewtoth. I missed this message completely.
> I still think we would want to abandon the work queue if we are interrupted instead of waiting for it to finish, no? My naive approach of not waiting for the queue to be empty does not work though.
Yeah, I donβt think thatβs safe. Other threads might be waiting on the tasks' futures to complete, so exiting without notifying them would leave them blocked forever.
What we could do (and I think you me
...
π¬ furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2172711323)
> Can we modify this to be more generic and return a type? And add logic to collect all returned values into a shared vector which can then be atomically swapped out by an observer? Possibly not in this PR, but if this will be split out into a generic thread pool.
We could keep track of the task futures' promises, if that's what you're referring to. In other words, this void() function is just a wrapper that executes a generic function which sets the result inside the callerβs future.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2172711323)
> Can we modify this to be more generic and return a type? And add logic to collect all returned values into a shared vector which can then be atomically swapped out by an observer? Possibly not in this PR, but if this will be split out into a generic thread pool.
We could keep track of the task futures' promises, if that's what you're referring to. In other words, this void() function is just a wrapper that executes a generic function which sets the result inside the callerβs future.
π¬ achow101 commented on pull request "blocks: force hash validations on disk read":
(https://github.com/bitcoin/bitcoin/pull/32638#issuecomment-3014267780)
ACK 9341b5333ad54ccdb7c16802ff06c51b956948e7
(https://github.com/bitcoin/bitcoin/pull/32638#issuecomment-3014267780)
ACK 9341b5333ad54ccdb7c16802ff06c51b956948e7
π achow101 merged a pull request: "blocks: force hash validations on disk read"
(https://github.com/bitcoin/bitcoin/pull/32638)
(https://github.com/bitcoin/bitcoin/pull/32638)
π¬ achow101 commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-3014283854)
ACK 95969bc58ae0cd928e536d7cb8541de93e8c7205
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-3014283854)
ACK 95969bc58ae0cd928e536d7cb8541de93e8c7205
π achow101 merged a pull request: "test: added fuzz coverage for consensus/merkle.cpp"
(https://github.com/bitcoin/bitcoin/pull/32243)
(https://github.com/bitcoin/bitcoin/pull/32243)
π¬ achow101 commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3014309449)
> `walletpassphrase` is currently the only RPC that schedules a new event, so all this PR will add to the queue are calls to `CWallet::Lock()` after the user needs a private key
`WalletContext` has its own `CScheduler` as well. I think it would probably make more sense to drop `HTTPRPCTimer` altogether and have the RPC schedule the lock by itself instead of taking a trip through the interfaces.
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3014309449)
> `walletpassphrase` is currently the only RPC that schedules a new event, so all this PR will add to the queue are calls to `CWallet::Lock()` after the user needs a private key
`WalletContext` has its own `CScheduler` as well. I think it would probably make more sense to drop `HTTPRPCTimer` altogether and have the RPC schedule the lock by itself instead of taking a trip through the interfaces.
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851169)
Removed
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851169)
Removed
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851273)
Made it >> 1
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851273)
Made it >> 1
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851365)
done
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851365)
done
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851409)
done
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851409)
done
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851511)
done
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851511)
done
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851580)
fixed
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851580)
fixed
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851840)
done
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172851840)
done
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172852325)
done
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172852325)
done
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172852584)
Yes! Removed, thanks
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172852584)
Yes! Removed, thanks
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172852782)
reworded
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2172852782)
reworded
π¬ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3014362470)
> See https://github.com/sipa/bitcoin/commits/pr31829
Thanks! I've incorporated this branch (squashed a couple of the changes directly into the main commit). Also addressed some comments. Still working on the bench and having `TxOrphanage` do its own limiting.
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3014362470)
> See https://github.com/sipa/bitcoin/commits/pr31829
Thanks! I've incorporated this branch (squashed a couple of the changes directly into the main commit). Also addressed some comments. Still working on the bench and having `TxOrphanage` do its own limiting.
π¬ achow101 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2172880709)
This comment is incorrect, it's not doing anything related to PSBT. You can just say that the scriptSigs and scriptWitnesses are being stripped in order to do a txid comparison.
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2172880709)
This comment is incorrect, it's not doing anything related to PSBT. You can just say that the scriptSigs and scriptWitnesses are being stripped in order to do a txid comparison.