[Merged by Bors] - ATX handler V2 supports marriages #6010

poszu · 2024-06-04T10:40:04Z

Motivation

Description

Implements the first step for merging ATXs - ID marriage.

Married IDs form an equivocation set. This set is identified by looking up all rows with the same marriage_atx column in the identities table.

The identities.IsMalicious() function was updated to check if any IDs that are part of the equivocation set are malicious (has identities.proof column populated).

For example, given below 2 rows, both Alice and Bob are malicious because they share marriage_atx and one of them has proof.

pubkey	proof	received	marriage_atx
ALICE	`<proof blob>`	1717500897643387339	`<ATX ID>`
BOB	NULL	0	`<ATX ID>`

The identities.EquivocationSet() function returns the complete set the given ID belongs to. For an ID that is not married, it returns just this ID as it forms a 1-element equivocation set.

Note: in the current implementation, an ID may be malicious but doesn't have a proof explicitly assigned - it could be assigned to a different ID in its set.

Test Plan

Added tests

TODO

Explain motivation or link existing issue(s)
Test changes and document test plan
Update documentation as needed
Update changelog as needed

codecov · 2024-06-05T11:08:19Z

Codecov Report

Attention: Patch coverage is 78.12500% with 28 lines in your changes missing coverage. Please review.

Project coverage is 81.4%. Comparing base (8df8e7f) to head (64efc3b).
Report is 13 commits behind head on develop.

Files	Patch %	Lines
activation/handler_v2.go	73.6%	10 Missing and 10 partials ⚠️
sql/identities/identities.go	87.7%	3 Missing and 3 partials ⚠️
activation/wire/wire_v2.go	50.0%	1 Missing ⚠️
malfeasance/wire/malfeasance.go	0.0%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6010     +/-   ##
=========================================
+ Coverage     81.3%   81.4%   +0.1%     
=========================================
  Files          296     299      +3     
  Lines        31632   32172    +540     
=========================================
+ Hits         25741   26219    +478     
- Misses        4210    4243     +33     
- Partials      1681    1710     +29

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

poszu · 2024-06-05T11:25:11Z

activation/handler_v2.go

+			proof := &mwire.MalfeasanceProof{
+				Proof: mwire.Proof{
+					Type: mwire.DoubleMarry,
+					Data: &mwire.DoubleMarryProof{},


Note: the proof must be implemented first to fill it here.

This would also create one proof for every identity with a certificate in the ATX which is not necessary. There should only be one proof that one identity married twice - and a list of certificates that all married that identity and become malicious too.

poszu · 2024-06-05T12:13:13Z

bors try

spacemesh-bors · 2024-06-05T12:58:32Z

try

Build succeeded:

malfeasance/handler.go

fasmat · 2024-06-05T21:44:46Z

malfeasance/wire/malfeasance.go

+type DoubleMarryProof struct {
+	// TODO: implement
+}
+
+func (p *DoubleMarryProof) isProof() {}
+


This belongs in the activation package.

I added the general process for this to #6043

fasmat · 2024-06-05T21:45:18Z

signing/signer.go

@@ -24,6 +24,7 @@ const (
 	BALLOT   = 2
 	HARE     = 3
 	POET     = 4
+	MARRIAGE = 5


Should this be its own domain? For me this belongs to ATX?

I'm not sure. The ATX domain is used for signing ATXs. The MARRIAGE domain is used to sign the marriage certs. It's included in an ATX but it's a different thing. I think there's no harm in it being a separate domain, is there?

fasmat · 2024-06-05T21:47:28Z

sql/identities/identities.go

+
+	rows, err := db.Exec(`
+	SELECT pubkey FROM identities
+	WHERE marriage_atx = (SELECT marriage_atx FROM identities WHERE pubkey = ?1) AND marriage_atx IS NOT NULL;`,


The second half seems redundant?

Suggested change

WHERE marriage_atx = (SELECT marriage_atx FROM identities WHERE pubkey = ?1) AND marriage_atx IS NOT NULL;`,

WHERE marriage_atx = (SELECT marriage_atx FROM identities WHERE pubkey = ?1);`,

This is required to handle a case when the ID is not married (marriage_atx = NULL) to avoid returning all non-married IDs from the DB. In this case rows == 0 and the function returns a single ID (non-married IDs form a set with just themselves).

Theoretically an identity could publish an ATX only containing a certificate signed by itself creating an equivocation set that only contains themselves. The "self marry" certificate is unfortunately required, but we could syntactically check that the MarriageCertificates a) contains at least 2 entries and b) only contains distinct entries.

…riages

fasmat · 2024-06-13T21:27:59Z

malfeasance/handler.go

@@ -44,6 +44,7 @@ const (
 	InvalidActivation MalfeasanceType = iota + 10
 	InvalidBallot
 	InvalidHareMsg
+	DoubleMarry = MalfeasanceType(wire.DoubleMarry)


Lets sync on this tomorrow and try to integrate - the double marry type isn't defined here. I outlined the basic structure here with the first malfeasance proofs: #6043

fasmat · 2024-06-13T21:30:58Z

activation/handler_v2.go

+			proof := &mwire.MalfeasanceProof{
+				Proof: mwire.Proof{
+					Type: mwire.DoubleMarry,
+					Data: &mwire.DoubleMarryProof{},


This would also create one proof for every identity with a certificate in the ATX which is not necessary. There should only be one proof that one identity married twice - and a list of certificates that all married that identity and become malicious too.

fasmat · 2024-06-13T22:46:01Z

activation/handler_v2_test.go

+		// TODO: check the proof contents once its implemented
+		require.NotNil(t, proof)


I think it would be better here not to return the proof but rather call a service (mock) with the instruction to publish the proof. I outlined the basic idea as part of this PR: #6043

activation/handler_v2_test.go

fasmat · 2024-06-13T22:57:22Z

activation/handler_v2.go

+	proof, err := h.checkDoubleMarry(tx, watx)
+	if err != nil {
+		return false, nil, fmt.Errorf("checking double marry: %w", err)
+	}
+	if proof != nil {
+		return true, proof, nil
+	}
+


I don't think we should return the proof here and bubble it up all the way until the handler to then publish and persist it. Instead after successfully persisting the (syntactically valid) ATX. We check for malfeasance and just and publish a proof when we have one - only reporting back to the caller that the check was positive.

activation/wire/wire_v2.go

fasmat · 2024-06-13T23:03:40Z

malfeasance/wire/malfeasance.go

+type DoubleMarryProof struct {
+	// TODO: implement
+}
+
+func (p *DoubleMarryProof) isProof() {}
+


I added the general process for this to #6043

fasmat · 2024-06-13T23:06:59Z

sql/identities/identities.go

+
+	rows, err := db.Exec(`
+	SELECT pubkey FROM identities
+	WHERE marriage_atx = (SELECT marriage_atx FROM identities WHERE pubkey = ?1) AND marriage_atx IS NOT NULL;`,


Theoretically an identity could publish an ATX only containing a certificate signed by itself creating an equivocation set that only contains themselves. The "self marry" certificate is unfortunately required, but we could syntactically check that the MarriageCertificates a) contains at least 2 entries and b) only contains distinct entries.

poszu · 2024-06-19T15:27:16Z

bors merge

## Motivation Closes #6007

poszu · 2024-06-19T15:37:33Z

bors cancel

spacemesh-bors · 2024-06-19T15:37:36Z

Canceled.

poszu · 2024-06-20T05:33:04Z

bors try

spacemesh-bors · 2024-06-20T06:16:05Z

try

Build failed:

systest-status

poszu · 2024-06-20T07:02:52Z

bors try

spacemesh-bors · 2024-06-20T07:53:31Z

try

Build failed:

systest-status

poszu · 2024-06-20T08:47:25Z

bors merge

## Motivation Closes #6007

spacemesh-bors · 2024-06-20T09:54:52Z

Build failed:

systest-status

poszu · 2024-06-20T14:12:12Z

Lots of GRPC errors in the test itself. Retrying

bors merge

## Motivation Closes #6007

spacemesh-bors · 2024-06-20T14:41:03Z

Build failed:

ci-status

poszu · 2024-06-20T15:25:34Z

Bors merge

## Motivation Closes #6007

spacemesh-bors · 2024-06-20T16:16:21Z

Pull request successfully merged into develop.

Build succeeded:

poszu added 4 commits June 4, 2024 12:29

ATX handler V2 supports marriages

d5ae0bf

ATX V2 - fix max marriages size

37c632a

Marrying malicious ID

cbbb881

Marrying when secondary ID is malicious

fb817dc

poszu force-pushed the atx-handler-v2/marriages branch from fda5153 to fb817dc Compare June 4, 2024 11:42

poszu added 4 commits June 4, 2024 15:59

ATX V2 - fix max marriages size

3258328

Add missing sql migration

b14b0d4

Satisfy linter

5fb8dca

Marry after checking malfeasance

6b7dd23

Merge branch 'develop' into atx-handler-v2/marriages

6a84f4c

poszu marked this pull request as ready for review June 5, 2024 11:23

poszu requested review from dshulyak, fasmat and ivan4th as code owners June 5, 2024 11:23

poszu commented Jun 5, 2024

View reviewed changes

spacemesh-bors bot added a commit that referenced this pull request Jun 5, 2024

Try #6010:

f02ee1b

fasmat reviewed Jun 5, 2024

View reviewed changes

poszu added 4 commits June 6, 2024 10:23

Fix DoubleMarry type

dde3be2

Merge remote-tracking branch 'origin/develop' into atx-handler-v2/mar…

a895310

…riages

Merge remote-tracking branch 'origin/develop' into atx-handler-v2/mar…

e263509

…riages

Merge remote-tracking branch 'origin/develop' into atx-handler-v2/mar…

41c4ea0

…riages

fasmat reviewed Jun 13, 2024

View reviewed changes

Review feedback: use table test

86dddd2

poszu requested a review from acud as a code owner June 17, 2024 09:01

ATX Signer must self-marry

302c9de

fasmat approved these changes Jun 19, 2024

View reviewed changes

Use reference ATX in marriage certificates

7f92ab8

spacemesh-bors bot pushed a commit that referenced this pull request Jun 19, 2024

ATX handler V2 supports marriages (#6010)

33395d0

## Motivation Closes #6007

Fix misspelling

64efc3b

spacemesh-bors bot added a commit that referenced this pull request Jun 20, 2024

Try #6010:

f714ffb

spacemesh-bors bot added a commit that referenced this pull request Jun 20, 2024

Try #6010:

e685f7d

fasmat approved these changes Jun 20, 2024

View reviewed changes

spacemesh-bors bot pushed a commit that referenced this pull request Jun 20, 2024

ATX handler V2 supports marriages (#6010)

65a23b0

## Motivation Closes #6007

spacemesh-bors bot pushed a commit that referenced this pull request Jun 20, 2024

ATX handler V2 supports marriages (#6010)

b268f5c

## Motivation Closes #6007

spacemesh-bors bot pushed a commit that referenced this pull request Jun 20, 2024

ATX handler V2 supports marriages (#6010)

a608922

## Motivation Closes #6007

spacemesh-bors bot changed the title ~~ATX handler V2 supports marriages~~ [Merged by Bors] - ATX handler V2 supports marriages Jun 20, 2024

spacemesh-bors bot closed this Jun 20, 2024

spacemesh-bors bot deleted the atx-handler-v2/marriages branch June 20, 2024 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - ATX handler V2 supports marriages #6010

[Merged by Bors] - ATX handler V2 supports marriages #6010

poszu commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 5, 2024 •

edited

Loading

poszu Jun 5, 2024

fasmat Jun 13, 2024

poszu commented Jun 5, 2024

spacemesh-bors bot commented Jun 5, 2024

fasmat Jun 5, 2024

fasmat Jun 13, 2024

fasmat Jun 5, 2024

poszu Jun 6, 2024

fasmat Jun 5, 2024

poszu Jun 6, 2024

fasmat Jun 13, 2024

fasmat Jun 13, 2024

fasmat Jun 13, 2024

fasmat Jun 13, 2024

fasmat Jun 13, 2024

fasmat Jun 13, 2024

fasmat Jun 13, 2024

poszu commented Jun 19, 2024

poszu commented Jun 19, 2024

spacemesh-bors bot commented Jun 19, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

	WHERE marriage_atx = (SELECT marriage_atx FROM identities WHERE pubkey = ?1) AND marriage_atx IS NOT NULL;`,
	WHERE marriage_atx = (SELECT marriage_atx FROM identities WHERE pubkey = ?1);`,

		// TODO: check the proof contents once its implemented
		require.NotNil(t, proof)

[Merged by Bors] - ATX handler V2 supports marriages #6010

[Merged by Bors] - ATX handler V2 supports marriages #6010

Conversation

poszu commented Jun 4, 2024 • edited Loading

Motivation

Description

Test Plan

TODO

codecov bot commented Jun 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poszu commented Jun 5, 2024

spacemesh-bors bot commented Jun 5, 2024

try

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poszu commented Jun 19, 2024

poszu commented Jun 19, 2024

spacemesh-bors bot commented Jun 19, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

try

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

try

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 20, 2024

spacemesh-bors bot commented Jun 20, 2024

poszu commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 5, 2024 •

edited

Loading