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.

Sign up for GitHub

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

chore: move BLS Sigs import to Rust Dash Core #2252

Merged
merged 1 commit into from
Oct 17, 2024
Merged

chore: move BLS Sigs import to Rust Dash Core #2252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions packages/rs-dpp/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ authors = [
anyhow = { version = "1.0.81" }
async-trait = { version = "0.1.79" }
base64 = "0.22.1"
bls-signatures = { git = "https://github.com/dashpay/bls-signatures", tag = "v1.3.1", optional = true }
bs58 = "0.5"
byteorder = { version = "1.4" }
chrono = { version = "0.4.35", default-features = false, features = [
Expand All @@ -29,7 +28,9 @@ dashcore = { git = "https://github.com/dashpay/rust-dashcore", features = [
"rand",
"signer",
"serde",
], default-features = false, tag = "0.31.0" }
"bls",
"eddsa"
], default-features = false, tag = "0.32.0" }
env_logger = { version = "0.11" }
getrandom = { version = "0.2", features = ["js"] }
hex = { version = "0.4" }
Expand All @@ -56,7 +57,6 @@ platform-versioning = { path = "../rs-platform-versioning" }
platform-serialization = { path = "../rs-platform-serialization" }
platform-serialization-derive = { path = "../rs-platform-serialization-derive" }
derive_more = { version = "1.0", features = ["from", "display"] }
ed25519-dalek = { version = "2.1", features = ["rand_core"], optional = true }
nohash-hasher = "0.2.0"
rust_decimal = "1.29.1"
rust_decimal_macros = "1.29.1"
Expand All @@ -75,6 +75,8 @@ once_cell = "1.7"

[features]
default = ["platform-value", "state-transitions"]
bls-signatures = ["dashcore/bls"]
ed25519-dalek = ["dashcore/eddsa"]
all_features = [
"json-object",
"platform-value",
Expand Down
2 changes: 1 addition & 1 deletion packages/rs-dpp/src/bls/native_bls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{BlsModule, ProtocolError, PublicKeyValidationError};
use anyhow::anyhow;
use bls_signatures::{PrivateKey, PublicKey};
use dashcore::bls_signatures::{self, PrivateKey, PublicKey};

#[derive(Default)]
pub struct NativeBlsModule;
Expand Down
2 changes: 1 addition & 1 deletion packages/rs-dpp/src/identity/identity_public_key/key_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use dashcore::secp256k1::rand::rngs::StdRng as EcdsaRng;
#[cfg(feature = "random-public-keys")]
use dashcore::secp256k1::rand::SeedableRng;
use dashcore::secp256k1::Secp256k1;
use dashcore::Network;
use dashcore::{bls_signatures, ed25519_dalek, Network};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Make imports of bls_signatures and ed25519_dalek conditional on feature flags

The bls_signatures and ed25519_dalek modules are only used when the corresponding features (bls-signatures and ed25519-dalek) are enabled. Including these imports unconditionally can lead to compilation errors when these features are not enabled. To prevent this, wrap the imports with the appropriate #[cfg(feature = "...")] attributes.

Apply this diff to conditionally import the modules:

- use dashcore::{bls_signatures, ed25519_dalek, Network};
+ #[cfg(feature = "bls-signatures")]
+ use dashcore::bls_signatures;
+ #[cfg(feature = "ed25519-dalek")]
+ use dashcore::ed25519_dalek;
+ use dashcore::Network;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use dashcore::{bls_signatures, ed25519_dalek, Network};
#[cfg(feature = "bls-signatures")]
use dashcore::bls_signatures;
#[cfg(feature = "ed25519-dalek")]
use dashcore::ed25519_dalek;
use dashcore::Network;

use itertools::Itertools;
use lazy_static::lazy_static;

Expand Down
2 changes: 1 addition & 1 deletion packages/rs-dpp/src/identity/identity_public_key/v0/methods/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use anyhow::anyhow;
use dashcore::hashes::Hash;
use dashcore::key::Secp256k1;
use dashcore::secp256k1::SecretKey;
use dashcore::{Network, PublicKey as ECDSAPublicKey};
use dashcore::{bls_signatures, ed25519_dalek, Network, PublicKey as ECDSAPublicKey};
use platform_value::Bytes20;

impl IdentityPublicKeyHashMethodsV0 for IdentityPublicKeyV0 {
Expand Down
6 changes: 3 additions & 3 deletions packages/rs-dpp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ pub mod prelude {

pub use bincode;
#[cfg(all(not(target_arch = "wasm32"), feature = "bls-signatures"))]
pub use bls_signatures;
pub use dashcore::bls_signatures;
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Direct usage of bls_signatures detected outside of dashcore.

Several instances of bls_signatures are still being used directly in the codebase. To maintain consistency and align with the objective of centralizing imports through dashcore::bls_signatures, please ensure that all usages are routed accordingly.

Affected Files:

  • packages/rs-dpp/src/signing.rs
  • packages/rs-dpp/src/identity/identity_public_key/key_type.rs
  • packages/rs-drive-abci/src/platform_types/commit/mod.rs
  • packages/rs-drive-proof-verifier/src/verify.rs
  • packages/simple-signer/src/signer.rs
  • (and others as indicated by the search results)
🔗 Analysis chain

LGTM: BLS signatures import moved to dashcore.

This change aligns with the PR objective of moving the BLS Signatures import to Rust Dash Core. It centralizes the import through the dashcore crate while maintaining the existing feature flag and architecture conditions.

Let's verify the usage of this new import path in the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new BLS signatures import path

# Test: Search for uses of bls_signatures from dashcore
rg --type rust 'use (crate::)?dashcore::bls_signatures'

# Test: Search for any remaining direct uses of bls_signatures
rg --type rust 'use bls_signatures'

Length of output: 153

Script:

#!/bin/bash
# Description: Comprehensive verification of bls_signatures usage in the codebase

# Test 1: Search for any occurrences of bls_signatures in the codebase
rg --type rust 'bls_signatures' --context 2

# Test 2: Ensure that all imports of bls_signatures are through dashcore
rg --type rust 'use\s+bls_signatures[^;]*;' | grep -v 'dashcore::bls_signatures'

# Test 3: Verify that there are no direct usages without the dashcore namespace
rg --type rust 'bls_signatures::' | grep -v 'dashcore::bls_signatures::'

Length of output: 30796

#[cfg(feature = "ed25519-dalek")]
pub use dashcore::ed25519_dalek;
#[cfg(feature = "system_contracts")]
pub use data_contracts;
#[cfg(feature = "ed25519-dalek")]
pub use ed25519_dalek;
#[cfg(feature = "jsonschema")]
pub use jsonschema;
pub use platform_serialization;
Expand Down
2 changes: 1 addition & 1 deletion packages/rs-dpp/src/signing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::serialization::PlatformMessageSignable;
use crate::validation::SimpleConsensusValidationResult;
#[cfg(feature = "message-signing")]
use crate::{BlsModule, ProtocolError};
use dashcore::signer;
use dashcore::{bls_signatures, signer};

impl PlatformMessageSignable for &[u8] {
#[cfg(feature = "message-signature-verification")]
Expand Down
2 changes: 1 addition & 1 deletion packages/rs-drive-abci/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ rand = "0.8.5"
tempfile = "3.3.0"
hex = "0.4.3"
indexmap = { version = "2.2.6", features = ["serde"] }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore-rpc", tag = "v0.15.7" }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore-rpc", tag = "v0.15.8" }
dpp = { path = "../rs-dpp", features = ["abci"] }
simple-signer = { path = "../simple-signer" }
rust_decimal = "1.2.5"
Expand Down
6 changes: 3 additions & 3 deletions packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ mod tests {

use dpp::consensus::ConsensusError;
use dpp::dashcore::secp256k1::Secp256k1;
use dpp::dashcore::{key::KeyPair, signer, Network, PrivateKey};
use dpp::dashcore::{key::Keypair, signer, Network, PrivateKey};

use dpp::data_contract::accessors::v0::{DataContractV0Getters, DataContractV0Setters};
use dpp::data_contract::document_type::random_document::{
Expand Down Expand Up @@ -2659,7 +2659,7 @@ mod tests {

let secp = Secp256k1::new();

let new_key_pair = KeyPair::new(&secp, &mut rng);
let new_key_pair = Keypair::new(&secp, &mut rng);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor duplicated key pair generation code

The line let new_key_pair = Keypair::new(&secp, &mut rng); is duplicated in both the identity_update_with_non_master_key_check_tx and identity_update_with_encryption_key_check_tx test functions. Consider extracting this code into a helper function or a test utility to reduce duplication and improve maintainability.

Also applies to: 2763-2763


let mut new_key = IdentityPublicKeyInCreationV0 {
id: 2,
Expand Down Expand Up @@ -2760,7 +2760,7 @@ mod tests {

let platform_state = platform.state.load();

let new_key_pair = KeyPair::new(&secp, &mut rng);
let new_key_pair = Keypair::new(&secp, &mut rng);

let new_key = IdentityPublicKeyInCreationV0 {
id: 2,
Expand Down
4 changes: 2 additions & 2 deletions ...e-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ mod tests {
use dpp::block::block_info::BlockInfo;
use dpp::consensus::ConsensusError;
use dpp::dash_to_credits;
use dpp::dashcore::key::{KeyPair, Secp256k1};
use dpp::dashcore::key::{Keypair, Secp256k1};
use dpp::data_contract::accessors::v0::DataContractV0Getters;
use dpp::identity::accessors::IdentityGettersV0;
use dpp::identity::contract_bounds::ContractBounds;
Expand Down Expand Up @@ -371,7 +371,7 @@ mod tests {

let mut rng = StdRng::seed_from_u64(1292);

let new_key_pair = KeyPair::new(&secp, &mut rng);
let new_key_pair = Keypair::new(&secp, &mut rng);

let new_key = IdentityPublicKeyInCreationV0 {
id: 2,
Expand Down
2 changes: 1 addition & 1 deletion packages/rs-sdk/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ envy = { version = "0.4.2", optional = true }
futures = { version = "0.3.30" }
derive_more = { version = "1.0", features = ["from"] }
# dashcore-rpc is only needed for core rpc; TODO remove once we have correct core rpc impl
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore-rpc", tag = "v0.15.7" }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore-rpc", tag = "v0.15.8" }
lru = { version = "0.12.3", optional = true }
bip37-bloom-filter = { git = "https://github.com/dashpay/rs-bip37-bloom-filter", branch = "develop" }
zeroize = { version = "1.8", features = ["derive"] }
Expand Down
2 changes: 1 addition & 1 deletion packages/simple-signer/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ rust-version.workspace = true

[dependencies]
bincode = { version = "2.0.0-rc.3", features = ["serde"] }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore-rpc", tag = "v0.15.7" }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore-rpc", tag = "v0.15.8" }
dpp = { path = "../rs-dpp", features = ["abci"] }
base64 = { version = "0.22.1" }
Loading