Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(drive)!: verify instant lock signatures with Drive #1875

Merged
merged 51 commits into from
Jun 23, 2024

Conversation

shumkov
Copy link
Member

@shumkov shumkov commented Jun 5, 2024

Issue being fixed or feature implemented

Core RPC was never designed for public access and according to our tests is a weak point in the system. Under the load, instant lock signature verification takes seconds. Since users can trigger signature verification via invalid IdentityCreateTransition for free, this open a DoS attack vector.

What was done?

  • Own implementation of Instant Lock signature verification for InstantAssetLockProof without calling Core RPC
  • Introduced support for DIP24 rotating quorums
  • Base config network in dashmate changed to mainnet
  • Updated quorums configuration for local, testnet and mainnet
  • Added missing logging for invalid asset lock based state transitions
  • Updated invalid signature error for better verbosity
  • Fixed internal error if chain lock signature is not valid

How Has This Been Tested?

  • Unit tests skip instant lock signature verification with disable_instant_lock_signature_verification test config option.
  • Implemented Instant Lock quorums and signing in strategy tests
  • Updated run_chain_top_up_identities to use InstantLock singing with DIP24 rotating quorums
  • Updated run_chain_insert_one_new_identity_per_block_with_block_signing to use InstantLock singing with DIP8 classic quorums
  • With test suite against local network
  • With test suite against a devnet

Breaking Changes

InstantAssetLockProof might give different validation results, since our implementation supports only recent signatures, compared with Core RPC. This means the previous blockchain data might become invalid.

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 added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@shumkov shumkov marked this pull request as ready for review June 7, 2024 07:42
@shumkov shumkov requested a review from QuantumExplorer as a code owner June 7, 2024 07:42
@shumkov shumkov changed the title perf(drive): verify instant lock signatures with Drive perf(drive): verify instant lock signatures with Drive [WIP] Jun 7, 2024
@shumkov shumkov changed the base branch from refactor/validating-quorums to v1.0-dev June 7, 2024 07:43
@shumkov shumkov changed the title perf(drive): verify instant lock signatures with Drive perf(drive)!: verify instant lock signatures with Drive Jun 14, 2024
@shumkov shumkov added this to the v1.0.0 milestone Jun 14, 2024
packages/rs-drive-abci/src/mimic/mod.rs Outdated Show resolved Hide resolved
Comment on lines 219 to 226
self.update_quorums(
QuorumSetType::InstantLock(instant_lock_quorum_type),
block_platform_state,
platform_state,
&extended_quorum_list,
is_validator_set_updated,
core_block_height,
)?;
Copy link
Member

Choose a reason for hiding this comment

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

I would do this only if instant_lock_quorum_type is different to chain_lock_quorum_type right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed


use crate::platform_types::validator_set::v0::{ValidatorSetV0, ValidatorSetV0Getters};
use crate::platform_types::validator_set::v0::{
ValidatorSetV0, ValidatorSetV0Getters, ValidatorSetV0Setters,
Copy link
Member

Choose a reason for hiding this comment

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

unused

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@shumkov
Copy link
Member Author

shumkov commented Jun 23, 2024

Agreed with @QuantumExplorer that I will merge this PR and he will do a post review of the recent two changes later.

@shumkov shumkov merged commit a89d854 into v1.0-dev Jun 23, 2024
119 checks passed
@shumkov shumkov deleted the perf/drve/validate-is-locks branch June 23, 2024 12:13
Sign up for free to 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