Skip to content

enable fraud service to full & light nodes and extend abci.FraudProof with go-fraud Proof interface - #891

Merged
gupadhyaya merged 3 commits into
evstack:mainfrom
gupadhyaya:fraud_exchange
May 10, 2023
Merged

enable fraud service to full & light nodes and extend abci.FraudProof with go-fraud Proof interface#891
gupadhyaya merged 3 commits into
evstack:mainfrom
gupadhyaya:fraud_exchange

Conversation

@gupadhyaya

@gupadhyayagupadhyaya commented Apr 25, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

As part of go-fraud library implementation, this PR adds:

  • ProofService to full and light nodes
  • Extends abci.FraudProof interface with go-fraud Proof interface

Fixes#892#893#894

@codecov

codecovBot commented Apr 25, 2023•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Patch coverage: 57.89% and project coverage change: +54.67 🎉

Comparison is base (d6d030a) 0.00% compared to head (9eb4ad9) 54.67%.

Additional details and impacted files
@@ Coverage Diff @@## main #891 +/- ##
=========================================
+ Coverage 0 54.67% +54.67% 
=========================================
Files 0 68 +68 Lines 0 11011 +11011 =========================================
+ Hits 0 6020 +6020 - Misses 0 4119 +4119 - Partials 0 872 +872
Impacted FilesCoverage Δ
node/light.go0.00% <0.00%> (ø)
types/state_fraud_proof.go26.66% <26.66%> (ø)
node/full.go61.86% <78.57%> (ø)
node/header_exchange.go54.81% <100.00%> (ø)
node/proof_service_factory.go100.00% <100.00%> (ø)

... and 63 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@nashqueuenashqueue added the C:state-fraud-proofs Component: State Fraud Proofs (generation and validation) label Apr 27, 2023
@gupadhyayagupadhyaya mentioned this pull request May 1, 2023
Closed
tzdybal
tzdybal suggested changes May 2, 2023
View reviewed changes

@tzdybaltzdybal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new code needs tests.

Comment threadtypes/fraud_proof.go Outdated
Comment threadnode/proof_service_factory.go Outdated
Manav-Aggarwal
Manav-Aggarwal requested changes May 3, 2023
View reviewed changes

@Manav-AggarwalManav-Aggarwal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Left some comments.

Comment threadnode/proof_service_factory.go Outdated
Comment threadtypes/fraud_proof.go Outdated
Comment threadnode/full.go
Comment threadnode/proof_service_factory.go Outdated
Comment threadnode/header_exchange.go
@gupadhyaya

gupadhyaya commented May 5, 2023•
edited by nashqueue
Loading

Copy link
Copy Markdown
ContributorAuthor

This new code needs tests.

this PR only adds fraud-exchange service to full and light nodes and for that the graceful start and stop is already tested in other unit tests. the fraud-exchange functionality will be added in the next PR along with new tests to cover the exchange part. will that suffice?

tzdybal
tzdybal previously approved these changes May 8, 2023
View reviewed changes
Ganesha Upadhyaya added 2 commits May 9, 2023 07:35
Ganesha UpadhyayaGanesha Upadhyaya
enable fraud service to full & light nodes and extend abci.FraudProof…
deb5357
… with go-fraud Proof interface methods
rename to state fraud proof
remove new method and force interface binding
Ganesha UpadhyayaGanesha Upadhyaya
better naming factory method, calling parent marshal/unmarshal
8729f4a
@gupadhyaya
gupadhyaya requested a review from tzdybalMay 9, 2023 15:10
Manav-Aggarwal
Manav-Aggarwal approved these changes May 9, 2023
View reviewed changes

@Manav-AggarwalManav-Aggarwal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This was linked to issues May 9, 2023
Use ProofServcie in light node #893
Closed
Use ProofService in full node #894
Closed
@gupadhyaya
gupadhyaya merged commit 5bf92b8 into evstack:mainMay 10, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Manav-AggarwalManav-AggarwalManav-Aggarwal approved these changes
@nashqueuenashqueueAwaiting requested review from nashqueue
@tuxcanflytuxcanflyAwaiting requested review from tuxcanfly
@S1nusS1nusAwaiting requested review from S1nus
+1 more reviewer
@tzdybaltzdybaltzdybal approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C:state-fraud-proofsComponent: State Fraud Proofs (generation and validation)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use ProofService in full node Use ProofServcie in light node Add StateFraudProof proof type

4 participants

@gupadhyaya@tzdybal@Manav-Aggarwal@nashqueue