schnorr: simplify some signing math #2042
Conversation
We reuse the Bytes() function rather than duplicating its logic.
|
ACK 7693d7f Checked that the existing code is just a duplicate of |
| commitment := chainhash.TaggedHash( | ||
| chainhash.TagBIP0340Challenge, rBytes[:], pBytes, hash, | ||
| chainhash.TagBIP0340Challenge, R.X.Bytes()[:], pBytes, hash, |
There was a problem hiding this comment.
While from a code readability perspective this might be a desired change, I think this will cause the rBytes array to escape to the heap (meaning it would need to be garbage collected) while before the change it would've been kept on the stack.
Would be nice to confirm or deny that with a benchmark test...
There was a problem hiding this comment.
I could swear that Go actually has some tooling that can show explicitly what goes on the heap and what stays on the stack. I'll have a look.
There was a problem hiding this comment.
Right, I thought about that as well directly after sending my comment above. See here: https://medium.com/@trinad536/escape-analysis-in-golang-fc81b78f3550
There was a problem hiding this comment.
btcec % go build -v -gcflags -m ./schnorr
schnorr/signature.go:308:43: new([32]byte) does not escape
I think this means we're good?
FWIW:
go version
go version go1.21.4 darwin/arm64
There was a problem hiding this comment.
Hmm, I'm not very well versed in reading that escape analysis messages. But I would think you'd need to take a look at the escape analysis for the implementation of Bytes() itself to get a clearer picture.
There was a problem hiding this comment.
@guggero The call to Bytes()gets inlined:
schnorr/signature.go:308:43: inlining call to secp256k1.(*FieldVal).Bytes
That is why the escape analysis shows that the new allocation is happening within schnorrSign, on the stack.
There was a problem hiding this comment.
Ah cool, good to know. I guess I should run the escape analysis more often myself then, pretty interesting to see what the compiler actually ends up doing.
We reuse the Bytes() function rather than duplicating its logic.