Results 187 comments of Sebastian Falbesoner

@davidgumberg: Thanks for the detailed review! I took most of your suggestions (see above) and rebased on master.

Thanks for the reviews! The suggestions (https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1931016480, https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1994315843, https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1996031367 and https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1996031577) make sense, but since they are all nits (IMHO), I'll address them if I have to retouch.

@MarnixCroes > Using the default address type (bech32) for this makes the most sense I think, instead of the latest, no? P2TR addresses look similar enough to P2WPKH ones (with...

Thanks for reviewing @vasild! Force-pushed with your -proxy suggestion in https://github.com/bitcoin/bitcoin/pull/31343#discussion_r1853553795 and updated the PR description accordingly with test instructions.

> We should call `SECP256K1_CHECKMEM_UNDEFINE` ( > > https://github.com/bitcoin-core/secp256k1/blob/b307614401790850b48fb3ba878247290857a975/src/checkmem.h#L27 > > ) in `secp256k1_memclear` after clearing the memory. Reading cleared memory would clearly be a bug. Thanks, added that, and...

> @theStack Can you rebase this on top of musig which has introduced a few more code locations that need clearing? Sure, done. With only five lines changed in the...

> This one comes to my mind, too: > > https://github.com/bitcoin-core/secp256k1/blob/a88aa9350633c2d2472bace5c290aa291c7f12c9/src/util.h#L253-L254 Ah good catch, missed that (only `grep`ped for `memset` calls). Tackled now as well.

> If you want motivated, you could look at `git grep secp256k1_ge_set_gej.*(`. @sipa's recent comment in the MuSig PR really caught my attention. When I worked on the previous PR,...

> My guess is that what we do currently is useless on any modern compiler. I admit that I haven't looked at the compiler output, but I'd rather spend the...

For a first check on whether this PR fulfills its promise, I looked at the generated assembler code of `secp256k1_ec_seckey_verify`, as this function is particularly short and should clear out...