commitment: fix pointer comparison of family keys #53
Conversation
asset/asset.go
Outdated
| // IsEqual returns true if this family key is equivalent to the passed other | ||
| // family key. | ||
| func (f *FamilyKey) IsEqual(otherFamilyKey *FamilyKey) bool { | ||
| return f.Key.IsEqual(&otherFamilyKey.Key) && |
There was a problem hiding this comment.
Could move the nil checks here, since in Golang you don't run into a nil reference issue when you call this method on a nil pointer (only when you de-reference something like f.Key or otherFamilyKey.Key).
There was a problem hiding this comment.
ptal, also ended up a bit more complicated as I forgot to rebase and the FamilyKey changed slightly.
Ah, missed your comment sorry. Should I close this so we fix this bug in that PR? |
|
Ah, it was actually here: #27 (comment) No, I think it's fine to have this as a separate PR. |
bhandras
force-pushed
the
family-key-mismatch-fix
branch
from
1f4096d
to
8f83ac2
Compare
Signed-off-by: Andras Banki-Horvath <bhandras@gmail.com>
bhandras
force-pushed
the
family-key-mismatch-fix
branch
from
8f83ac2
to
bfc69b8
Compare
| @@ -81,7 +81,7 @@ func parseCommon(assets ...*asset.Asset) (*AssetCommitment, error) { | |||
| assetFamilyKey := assets[0].FamilyKey | |||
| assetsMap := make(map[[32]byte]*asset.Asset, len(assets)) | |||
| for _, asset := range assets { | |||
| if assetFamilyKey != asset.FamilyKey { | |||
There was a problem hiding this comment.
Nice catch! I think this was introduced when a change the family keys to be a nested struct, instead of just a normal btcec.PublicKey.
This PR fixes a bug where we compared pointers of family keys instead of their equivalency when creating asset commitments.