go-spacemesh icon indicating copy to clipboard operation
go-spacemesh copied to clipboard

WiP: Malfeasance v2: Handlers and Publishers

Open fasmat opened this issue 1 year ago • 1 comments

Motivation

This PR adds handlers and publishers for malfeasance v2 proofs. This is a pre-requisite for enabling v2 ATXs on mainnet.

Closes #6143

Description

For this the existing malfeasance handler has been simplified again to only handle v1 proofs, with the goal of eventually retiring v1 malfeasance proofs in a future release. For malfeasance v2 new handlers and publishers were added.

Dedicated publishers for v1 and v2 proofs have been added, shifting the responsibility for publishing proofs from the handlers of components (activation, hare etc.) to dedicated services. This also allows to simplify handling proof publication since the components now don't need to keep track any more if they are in sync or not before publishing a proof (the publisher now keeps track of this instead).

The new DB table malfeasance requires foreign key constraints so these have now been enabled using PRAGMA foreign_keys = ON;. The new table will keep track of malicious identities and their proofs. The existing identities table will in future only be used to keep track of which identities belong to which marriage set. The proofs in them will be dropped when the publisher / handler of malfeasance v1 proofs are retired and existing proofs will be migrated to V2.

The general pattern for handling malfeasance v2 proofs is:

  1. receive proof via sync or gossip (malfeasance hander)
    • if the sender of the proof is self (because we just published the proof) return without doing anything (proof is already validated and persisted during publish)
  2. call domain specific malfeasance handler to validate the proof for the identity marked as malicious by the proof (e.g. atx malfeasance handler)
  3. if proof is valid evaluate the provided certificates to be valid (in the general malfeasance handler)
  4. store proof for the given identity and update the equivocation set to contain all identities that were not known to be part of the set before

The general pattern for publishing a malfeasance proof is:

  1. in some domain malicious behavior is detected and a domain specific proof is created for it (eg. double marry in activation domain)
  2. proof is passed to the domain specific publisher, which validates the proof to ensure that if we publish it others will also consider it valid
  3. domain specific publisher passes the proof along to the general malfeasance publisher that stores the proof
  4. if the node is in sync the general publisher publishes the proof to the network via libp2p (otherwise it is only persisted)

Test Plan

  • existing tests were updated
  • tests have been added for the new functionalities

TODO

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

fasmat avatar Aug 29 '24 19:08 fasmat

Codecov Report

Attention: Patch coverage is 84.83755% with 84 lines in your changes missing coverage. Please review.

Project coverage is 79.9%. Comparing base (4a8beae) to head (5c95012). Report is 1 commits behind head on develop.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
malfeasance2/publisher.go 64.3% 32 Missing and 14 partials :warning:
malfeasance2/handler.go 85.3% 20 Missing and 9 partials :warning:
activation/malfeasance2.go 95.7% 2 Missing and 1 partial :warning:
sql/marriage/marriages.go 86.3% 2 Missing and 1 partial :warning:
tortoise/state.go 40.0% 2 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #6307     +/-   ##
=========================================
- Coverage     79.9%   79.9%   -0.1%     
=========================================
  Files          356     358      +2     
  Lines        47477   47999    +522     
=========================================
+ Hits         37967   38378    +411     
- Misses        7362    7446     +84     
- Partials      2148    2175     +27     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 29 '24 19:08 codecov[bot]

The new DB table malfeasance requires foreign key constraints so these have now been enabled using PRAGMA foreign_keys = ON;.

This may have quite serious performance impact. The constraints are enabled globally and, in particular, they will affect INSERTs on all the tables, as SQLite will make internal queries to verify the validity of each inserted row. This may mean substantial ATX ingestion performance degradation during ATX storm, among other possible issues. I'd advice testing using syncv1 against a single peer or active syncv2 using a database snapshot missing the last epoch and compare ATX ingestion rate (possibly with verification disabled to check DB performance on its own).

Maybe it would be better to do manual constraint checks for the malfeasance table?

ivan4th avatar Jan 15 '25 12:01 ivan4th

I don't see foreign key constraints being enabled in the code, however, is it outdated description or is smth missing?

ivan4th avatar Jan 15 '25 12:01 ivan4th

I don't see foreign key constraints being enabled in the code, however, is it outdated description or is smth missing?

They were necessary when I started working on this task to ensure that the data in the malfeasance table actually references only existing marriage data and is updated when needed. At some point I changed this from foreign key constraints (that didn't work as I wanted them with sql lite anyway) to triggers. So yes this section of the PR description can be ignored.

fasmat avatar Jan 15 '25 14:01 fasmat

bors merge

fasmat avatar Jan 15 '25 17:01 fasmat

Build failed:

spacemesh-bors[bot] avatar Jan 15 '25 18:01 spacemesh-bors[bot]

bors merge

fasmat avatar Jan 15 '25 18:01 fasmat

Pull request successfully merged into develop.

Build succeeded:

spacemesh-bors[bot] avatar Jan 15 '25 19:01 spacemesh-bors[bot]