Skip to content

feat(rs-drive-abci)!: commit and vote extensions signature verification - #915

Merged
QuantumExplorer merged 20 commits into
feat/abci_validationfrom
feat/commit-validation
Apr 18, 2023
Merged

feat(rs-drive-abci)!: commit and vote extensions signature verification#915
QuantumExplorer merged 20 commits into
feat/abci_validationfrom
feat/commit-validation

Conversation

@lklimek

@lklimeklklimek commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

We want to check if digital signatures returned by Tednerdash are valid.

What was done?

  1. Created Commit struct with verify_signature
  2. Fix: ignore empty signatures when comparing Withdrawal Txs
  3. Implemented verify_signatures() for WithdrawalTxs
  4. Implemented unit tests
  5. Companion PR: feat(proto): add sign_digest() to commit and vote extension rs-tenderdash-abci#20

How Has This Been Tested?

Added unit tests that test vectors generated in Tenderdash

Breaking Changes

Added new configuration option: CHAIN_ID

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lklimek
lklimek marked this pull request as draft April 14, 2023 11:41
@lklimeklklimek changed the title feat(rs-drive-abci): verification of commi signaturefeat(rs-drive-abci)!: verification of commi signatureApr 14, 2023
@lklimeklklimek changed the title feat(rs-drive-abci)!: verification of commi signaturefeat(rs-drive-abci)!: verification of commit signatureApr 14, 2023
@lklimeklklimek changed the title feat(rs-drive-abci)!: verification of commit signaturefeat(rs-drive-abci)!: verification of commit and vote extension signaturesApr 17, 2023
@lklimeklklimek changed the title feat(rs-drive-abci)!: verification of commit and vote extension signaturesfeat(rs-drive-abci)!: commit and vote extensions signature verificationApr 17, 2023
@lklimek
lklimek marked this pull request as ready for review April 17, 2023 15:28

/// The default quorum type
pub quorum_type: QuorumType,
pub quorum_type: String,

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.

Why have this as a String?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

because there is a bug in envy / serde, and flattened structs don't deserialize correctly into enums, ints, etc. softprops/envy#26

Comment on lines +416 to +423
let validation_result = received_withdrawals.verify_signatures(
&self.config.abci.chain_id,
self.config.quorum_type(),
quorum_hash,
height,
round,
validator_public_key,
);

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.

I think this should return a validation result instead of a result

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Returning SimpleValidationResult, moved part of logic to received_withdrawals.verify_signatures

Comment threadpackages/rs-drive-abci/src/execution/engine.rs Outdated
Comment threadpackages/rs-drive-abci/src/abci/withdrawal.rs Outdated
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@lklimek@QuantumExplorer