Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 cobratbq commented on issue "Is not demanding low-R grinding still relevant for external signers?":
(https://github.com/bitcoin/bitcoin/issues/33761#issuecomment-3478379562)
This seems to do the job. Right, so high-`s` was the problem. Given that it produces signatures of both 222 bytes and 223 bytes, high `R` must indeed (still) be accepted.
cobratbq closed an issue: "Is not demanding low-R grinding still relevant for external signers?"
(https://github.com/bitcoin/bitcoin/issues/33761)
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485033847)
I did try and use a sorted vector and do binary search in the workers, but it was not a measurable performance difference. In theory it should be faster since txid comparison is much faster than siphash. I can do this if you want, but I think it makes the code clearer using the `unordered_set`.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485039220)
This is a consequence of skipping the main cache and writing directly to the temp cache.

I don't think we can modify `GetCoin` to not return spent coins, it's part of the definition [Retrieve the Coin (unspent transaction output) for a given outpoint.](https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L310). This also goes backwards from your change https://github.com/bitcoin/bitcoin/pull/30849 where you added TODOs to not return spent coins where our test `GetCoin`s do. I don't thin
...
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485040051)
It was for false sharing. I don't think it has a measurable effect, but it might on some systems? I think it's harmless to keep, since there's only one InputFetcher.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485040642)
I don't think that's actually preferable. The current way is more readable IMO.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485042527)
There are no locks... This is a lock free implementation. It is synchronized with atomics per input, and the threads are work stealing via the `m_input_head`. Or do you mean something else?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485043525)
We can't have the main thread insert a failed fetch. The coin will not be updated. I suppose the main thread could check if the coin is unspent.
We want to exit ASAP as well if we can't find an input, so we need to signal the main thread that they should exit the loop.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485058898)
In `ConnectBlock` if we encounter a missing input we abort validation immediately. It is wasted work to continue. Why should we do it here?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485063519)
Right, didn't cover this new happy path in tests. Unlikely case is covered though. Will add. Thanks.
👍 andrewtoth approved a pull request: "[IBD] coins: reduce lookups in dbcache layer propagation"
(https://github.com/bitcoin/bitcoin/pull/33602#pullrequestreview-3409084819)
utACK 0ac969cddfdba52f7947e9b140ef36e2b19c2c41

I did not verify the speedup, but it is a nice optimization that will benefit every `Flush`.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485073051)
`<cstdint>` we need because we use `int32_t`.
💬 cobratbq commented on issue "External signer: in case of error shows only "External signer failure"":
(https://github.com/bitcoin/bitcoin/issues/31004#issuecomment-3478461277)
This has indeed improved in later version. I am currently running v29.2.0 which does show output.
Personally, I think the current implementation does not take into account the cases where there is excessive output, e.g. logging entries. Instead, the first line is assumed to be relevant information concerning the error. Arguably, in such a case, the last line would be better.

It is not a serious problem. I might start stderr output with a line that refers to stderr-output and/or logging.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485073570)
Yes, I need to fuzz it though myself. The CI is fuzzing it a little bit.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485074194)
This won't have a measurable effect if we are connecting lots of blocks though.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485074795)
We could just get rid of this line now.
📝 cobratbq opened a pull request: "doc: update interface, --stdin flag, IPC-command signtx (#31005)"
(https://github.com/bitcoin/bitcoin/pull/33765)
Updates to documentation for External Signer.

- Added mention that `signtransaction` command is no longer primary mechanism.
- Document IPC mechanism via `--stdin` flag followed with stdin-content.
- Document `signtx` IPC-command followed by Base64-encoded PSBT.
💬 cobratbq commented on issue "Docs: "External signer" documentation is outdated. (plz close if unwanted request)":
(https://github.com/bitcoin/bitcoin/issues/31005#issuecomment-3478486581)
@Sjors feel free to review my PR #33765 :-P
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485086517)
Got rid of it.