btcec: add deep copy method for public keys

[ziggie1984]

We use a lot of pointers to the PublicKey type, so it comes very handy to have a deep copy method available.

[coveralls]

Pull Request Test Coverage Report for Build 12273833480

Details

• 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.004%) to 57.284%

---

Files with Coverage Reduction	New Missed Lines	%
peer/peer.go	1	74.52%

Totals
Change from base Build 12253655920:	0.004%
Covered Lines:	29917
Relevant Lines:	52226

---

💛 - Coveralls

[guggero]

LGTM 🎉

[guggero]

nit: use var pk secp.JacobianPoint instead?

[Roasbeef]

I don't think we want to copy things this way. You need to call ToAffine when coming back from the Jacobian coords.

Here's some code examples:

• https://github.com/decred/dcrd/blob/692ec68023fb27eca7d5d32801202eed2d86a5c9/hdkeychain/extendedkey.go#L333-L340
• https://github.com/decred/dcrd/blob/692ec68023fb27eca7d5d32801202eed2d86a5c9/dcrec/secp256k1/privkey.go#L87-L93

• btcd/btcec/btcec.go

  Lines 44 to 55 in e646d43

  	func Generator() *PublicKey {
  	var (
  	result JacobianPoint
  	k secp.ModNScalar
  	)
  	k.SetInt(1)
  	ScalarBaseMultNonConst(&k, &result)
  	result.ToAffine()
  	return NewPublicKey(&result.X, &result.Y)

After computations, we always transport/handle in affine form.

[Roasbeef]

I think we should add a property based test here. I wager it would've caught this issue.

The property we care about here is: pub == copy(pub).

If you look at what ToAffine does, it actually will arrive at new values for x and y, as it's a distinct coordinate system: https://github.com/decred/dcrd/blob/08d8572807872f2b9737f8a118b16c320a04b077/dcrec/secp256k1/curve.go#L136-L149

[guggero]

I thought the same about ToAffine(), but since we use AsJacobian() right before that, those should be affine coordinates already or does that no hold?

[ziggie1984]

it would still work because we are not doing any operations on it so z is set to 1 anyways and Affine and Jacobian are the same coordinates but technically it's wrong but it just happens to work for this particular cpy method. Will correct it.

[Roasbeef]

I don't think we want to copy things this way. You need to call ToAffine when coming back from the Jacobian coords.

Here's some code examples:

• https://github.com/decred/dcrd/blob/692ec68023fb27eca7d5d32801202eed2d86a5c9/hdkeychain/extendedkey.go#L333-L340
• https://github.com/decred/dcrd/blob/692ec68023fb27eca7d5d32801202eed2d86a5c9/dcrec/secp256k1/privkey.go#L87-L93

• btcd/btcec/btcec.go

  Lines 44 to 55 in e646d43

  	func Generator() *PublicKey {
  	var (
  	result JacobianPoint
  	k secp.ModNScalar
  	)
  	k.SetInt(1)
  	ScalarBaseMultNonConst(&k, &result)
  	result.ToAffine()
  	return NewPublicKey(&result.X, &result.Y)

After computations, we always transport/handle in affine form.

[guggero]

Nice, LGTM 🎉