universe: fix group key proof/leaf comparison

[jtobin]

Found by chance while reviewing #1784 -- we want to return an error if the two are unequal, not equal.

[jtobin]

Interestingly the supply_commit_ignore_asset itest seems to fail due to this "fix." Let me take another look.

[coveralls]

Pull Request Test Coverage Report for Build 17604098062

Details

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• 77 unchanged lines in 17 files lost coverage.
• Overall coverage decreased (-0.03%) to 56.958%

---

Files with Coverage Reduction	New Missed Lines	%
commitment/tap.go	1	85.65%
asset/group_key.go	2	72.15%
tapdb/addrs.go	2	78.23%
tapdb/sqlc/universe.sql.go	2	75.78%
tapdb/universe.go	2	81.11%
universe_rpc_diff.go	2	76.0%
universe/syncer.go	2	82.3%
asset/asset.go	3	80.21%
proof/verifier.go	3	87.54%
tapchannel/aux_leaf_signer.go	3	43.53%

Totals
Change from base Build 17562325369:	-0.03%
Covered Lines:	63161
Relevant Lines:	110891

---

💛 - Coveralls

[jtobin]

Looks like the itest passes if the comparison is changed to a deep equality check, which seems to make sense, as there seems to be no reason that the Asset in the issuanceProof should be the same as that indirectly pointed to via GroupKey in the issuanceLeaf.

[ffranr]

Interestingly the supply_commit_ignore_asset itest seems to fail due to this "fix." Let me take another look.

Well spotted, thanks! I think this is the diff we need:

Index: universe/supplyverifier/verifier.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/universe/supplyverifier/verifier.go b/universe/supplyverifier/verifier.go
--- a/universe/supplyverifier/verifier.go	(revision 306bf3588f592e04cecb8608d55d592be87875cf)
+++ b/universe/supplyverifier/verifier.go	(date 1757490882205)
@@ -423,7 +423,7 @@
 
 	_, err = issuanceProof.Verify(ctx, nil, lookup, vCtx)
 	if err != nil {
-		return fmt.Errorf("burn leaf proof failed verification: %w",
+		return fmt.Errorf("issuance leaf proof failed verification: %w",
 			err)
 	}
 
@@ -440,7 +440,7 @@
 	}
 
 	if issuanceLeaf.IsBurn {
-		return fmt.Errorf("IsBurn is enexpectedly true for issuance " +
+		return fmt.Errorf("IsBurn is unexpectedly true for issuance " +
 			"leaf")
 	}
 
@@ -463,7 +463,9 @@
 		return fmt.Errorf("missing group key in issuance leaf")
 	}
 
-	if issuanceProof.Asset.GroupKey == issuanceLeaf.GroupKey {
+	if issuanceProof.Asset.GroupKey.GroupPubKey !=
+		issuanceLeaf.GroupKey.GroupPubKey {
+
 		return fmt.Errorf("group key in issuance leaf does not match " +
 			"group key in issuance proof")
 	}

There are likely unset fields in GroupKey. But we really only care about GroupKey.GroupPubKey. Itest passes with this diff.

@jtobin shall i just add a commit to #1784 and we close this PR? Then I'll have this change in WIP PR #1777

[jtobin]

There are likely unset fields in GroupKey. But we really only care about GroupKey.GroupPubKey. Itest passes with this diff.

@jtobin shall i just add a commit to #1784 and we close this PR? Then I'll have this change in WIP PR #1777

Yeah makes sense, and I think a direct comparison is superior. Is there a need to compare the GroupKeyVersion as well?

(But otherwise, go for it. 👍)

[ffranr]

Is there a need to compare the GroupKeyVersion as well?

I don’t think so. Here we only need to ensure the issuance event fields consistently refer to the same asset group, and checking equality of the asset group’s public key is sufficient for that.

I've added the patch to #1784

Thanks for looking into this!