< sipa>
achow101: i'm breaking my head... i don't understand how ProduceSignature ever worked at all
< achow101>
sipa: where?
< sipa>
if you call it through SignSignature, from multiple SPKMs, as far as i can see, if the first SPKM is able to sign, the second one will just wipe the witness
< achow101>
It'll do VerifyScript and find it to be valid
< sipa>
oh
< achow101>
Then abort early and not change anything
< sipa>
got it!
< sipa>
this explains
< sipa>
thanks
< sipa>
it constructs a new checker objects, which doesn't have access to PrecomputedTransactionData, which is necessary to verify taproot sigs
< achow101>
Yeah
< achow101>
I have a pr for that actually
< sipa>
oh?
< achow101>
(for other reasons, but should give access to that)
< sipa>
passing down the signaturecreator's checker, rather than constructing a new one?
< achow101>
#21166
< gribble>
https://github.com/bitcoin/bitcoin/issues/21166 | Have SignatureExtractorChecker subclass MutableTransactionSignatureChecker by achow101 · Pull Request #21166 · bitcoin/bitcoin · GitHub
< sipa>
no, that doesn't work
< sipa>
it's still creating a new checker
< sipa>
(which includes a new MutableTransactionSignatureChecker)
< achow101>
Ah, think I just don't know what you need
< sipa>
yeah, it's a fine change, but it's orthogonal
< sipa>
also: really, we're verifying every existing witness in a transaction being signed in every SPKM?
< achow101>
I guess we are
< sipa>
i'm sure there's a way to keep it in SignatureData form across those calls
< sipa>
which would avoid that
< sipa>
also orthogonal, of course
< achow101>
Yeah. Signing performance hasn't really been a concern too
< sipa>
yeah
< sipa>
just seems a bit silly, it's not a big deal
< sipa>
achow101: can you explain how 21166 fixes 21151? what function specifically is missing?
< achow101>
CheckSequnce was missing
< sipa>
i see
< achow101>
Same with ChecLockTime, so op_cltv scripts don't work too
< sipa>
yeah
< sipa>
achow101: i think there is a simpler solution... just don't wipe scriptWitness
< sipa>
unless sigdata.witness is false
< achow101>
what about scriptSig?
< sipa>
there are a couple things
< sipa>
ideally just these functions don't get called at all for things that are already signed
< sipa>
the caller has the best context to determine that, i think
< sipa>
caller = wallet, rpc, psbt, ...
< achow101>
not necessarily
< sipa>
e.g. if you lack amount information, a VerifyScript will fail for every witness
< sipa>
that doesn't mean you should wipe it
< achow101>
for these non-wallet inputs, no context knows until it runs through the verifier
< sipa>
ok, setting aside the question of deciding whether an input is fully signed or not
< sipa>
if you somehow think it's not yet fully signed, so try to sign yourself, and fail... there is no point in wiping anything at all
< bitcoin-git>
[gui] luke-jr opened pull request #230: Support backup to new text-based database dump format (master...gui_backup_formats) https://github.com/bitcoin-core/gui/pull/230
< jonatack>
vasild: I finished reviewing the I2P PR late last night, then my internet went down, and when I restarted GitHub had saved none of the many comments I had pending. Disheartening. I'm thinking that by default now on large PR reviews I'll start annotating the code directly, git commit and push my own branch, then refer to the branch in my review. That also takes much less GitHub space,
< jonatack>
which is good in order to avoid deeper "hidden items" clicking, which for me is up to 105 hidden items in that PR, and which continued to hide one of your commits even after clicking repeatedly to unhide everything.
< jonatack>
(Not a reflection on the PR itself--it looks very good!)
< jeremyrubin>
if I make a PR, and then I make another PR referincing that PR in the title, and then I change the first to reference the latter, and then put the number in IRC what does gribble do?
< luke-jr>
I doubt gribble responds to itself
< luke-jr>
gribble: echo gribble: echo hi
< luke-jr>
x.x
< luke-jr>
jeremyrubin: also, you'd need to be quick - gribble doesn't announce renames ;)
< luke-jr>
err, maybe not relevant if you manually enter it
< luke-jr>
anyway, off topic XD
< phantomcircuit>
jeremyrubin, it ignores itself as all irc bots that aren't instantly banned do
< bitcoin-git>
[bitcoin] theStack opened pull request #21311: rpc: document optional fields for getchaintxstats result (master...2021-rpc-document_optional_getchaintxstats_fields) https://github.com/bitcoin/bitcoin/pull/21311
< bitcoin-git>
[bitcoin] vladyslavstartsev opened pull request #21312: wallet: remove lock during `listaddressgroupings` (master...listaddressgroupings-no-lock) https://github.com/bitcoin/bitcoin/pull/21312
< s7r>
building from source doesn't work any more cleanly on Debian.
< s7r>
I get A compiler with support for C++17 language features is required.
< s7r>
(current compiler does support C++17 though)