-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Newsletters: add 83 (2020-02-05) #334
Conversation
Pushed a small edit on the 16702 write-up (partially based on feedback from moneyball) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have time to do a detailed review this week, but I've left some very high-level feedback on the Proposed amendments to BIP340 schnorr signatures item. All the rest looks good from a quick skim.
I read the newsletter but didn't scrutinize it closely. LGTM! |
ACK 084b2a2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few suggestions before we go to press. Sorry, I'm late to review.
ACK after that.
b4a114a
to
b6724a6
Compare
Added a commit for @adamjonas feedback, thanks for those catches. Squashed and merged. |
Oh! This was merged earlier than usual. Sorry, I've been caught up reviewing for today's review club session I'm hosting. Will still have a look. |
However, there remains a risk that inexperienced developers will not | ||
perform this step, and so it seems preferable to use Jonas Nick's | ||
suggestion (relayed by Maxwell) of including the public key in the | ||
data used to produce a deterministic nonce. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just now on #secp256k1 irc:
08:05 <gmaxwell> https://bitcoinops.org/en/newsletters/2020/02/05/ is kinda odd
08:06 <gmaxwell> anyone ever find any implementation that verifies after in open source code in the wild?
08:06 <gmaxwell> I think it's strange to say " However, there remains a risk that inexperienced developers will not perform this step" regarding a pretty computationally expensive step that I don't ever recall seeing any implementation take.
08:08 <sipa> gmaxwell: agree
08:10 <sipa> harding: ping ^... perhaps it's better to say something like "However, the computational overhead of this approach may not be acceptable to many applications" or so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
08:17 <gmaxwell> (I think probably _we_ should do this in libsecp, optionally annd have it controllable with a context flag...because our signing is usually used in cases where performance doesn't matter)
08:33 <gmaxwell> I guess in bitcoin-core it kind of accidentally verifies after since it puts stuff in the mempool before relaying.
08:33 <gmaxwell> I think you could extract from the wallet via getraw a txn with a failing signature however.
08:41 <sipa> gmaxwell: no, even before that
08:41 <sipa> the script signing logic invokes validation
08:41 <sipa> to determine whether it's done signing
08:42 <sipa> though i'm not sure that failure at that level would e.g. not result in the invalid signature ending up in a PSBT or so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK, LGTM.
Humble possible suggestion for notable changes next week: bitcoin/bitcoin#17585 affects RPC API users by deprecating the getwalletinfo label
field, which has been superceded by labels
.
No description provided.