Skip to content

feat: migrate from bls-signatures to blsful library #82

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

Merged
PastaPastaPasta merged 1 commit into from
Jul 8, 2025
Merged

feat: migrate from bls-signatures to blsful library #82

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
3 changes: 3 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,8 @@ version = "0.39.6"
[patch.crates-io.dashcore_hashes]
path = "hashes"

[patch.crates-io]
blsful = { git = "https://github.com/dashpay/agora-blsful", rev = "5f017aa1a0452ebc73e47f219f50c906522df4ea" }
Comment on lines +11 to +12
Copy link
Member

@PastaPastaPasta PastaPastaPasta Jul 8, 2025

Choose a reason for hiding this comment

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

@coderabbitai is this the proper way to include this? especially the header "[patch.crates-io]"

Copy link
Contributor

@coderabbitai coderabbitai bot Jul 8, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

@PastaPastaPasta Yes, this is the proper way to include the patch! The [patch.crates-io] header is correct. Here's what's happening:

  1. Correct syntax: The [patch.crates-io] section is the standard way to override crates that would normally come from crates.io with alternative sources (like Git repositories).

  2. Multiple patch sections: You can have multiple patch sections in your Cargo.toml:

    • [patch.crates-io.dashcore_hashes] - This is shorthand for patching a specific crate
    • [patch.crates-io] - This is the general section where you can patch multiple crates

  3. Alternative approach: You could consolidate both patches into a single [patch.crates-io] section like this:

    [patch.crates-io]
dashcore_hashes = { path = "hashes" }
blsful = { git = "https://github.com/dashpay/agora-blsful", rev = "5f017aa1a0452ebc73e47f219f50c906522df4ea" }

Both approaches are valid, but the consolidated version is more commonly used. The current structure with separate sections works perfectly fine and is also valid Cargo syntax.

The patch ensures that whenever any crate in your workspace depends on blsful, it will use your specified Git revision instead of the version from crates.io.

ʕ•ᴥ•ʔ

🌐 Web query:

Cargo.toml patch.crates-io syntax multiple patch sections proper way

💡 Result:

In Cargo.toml, you can use the [patch.crates-io] section to override specific crates from crates.io with your own version (usually a local path or a git repository). However, there is a specific way to do this properly:

  1. Only one [patch.crates-io] section is allowed per Cargo.toml.
  2. Inside that section, you specify multiple crates to patch.
  3. Each crate you want to patch is listed as a key with the respective override (path, git, etc).

Example of multiple patches in one section:

[patch.crates-io]
serde = { git = "https://github.com/serde-rs/serde", branch = "master" }
serde_json = { path = "../local/serde_json" }

If you have multiple patches, you must put them all inside the same [patch.crates-io] block. Having multiple [patch.crates-io] blocks in a single Cargo.toml is not valid.

If you want to patch crates for different registries, you would use different [patch.<registry>] sections—but for crates.io patches, only one [patch.crates-io] section.

So the proper way to have multiple patches is:

[patch.crates-io]
crate1 = { path = "path_to_crate1" }
crate2 = { git = "https://github.com/user/crate2" }
crate3 = { version = "1.2.3", registry = "custom-registry" }

But remember the patch feature only overrides dependencies from crates.io by default, unless you specify otherwise with registries.

Would you like a sample Cargo.toml showcasing multiple patches?

@PastaPastaPasta I need to correct my previous response. Based on the Cargo documentation, the current setup is not the proper way to do this.

The Issue: You can only have one [patch.crates-io] section per Cargo.toml file. Having both [patch.crates-io.dashcore_hashes] and [patch.crates-io] sections is invalid.

The Correct Approach: You should consolidate both patches into a single [patch.crates-io] section:

[patch.crates-io]
dashcore_hashes = { path = "hashes" }
blsful = { git = "https://github.com/dashpay/agora-blsful", rev = "5f017aa1a0452ebc73e47f219f50c906522df4ea" }

Why this matters:

  • Cargo only allows one [patch.crates-io] section per file
  • All crates.io patches must be listed within that single section
  • The current structure may lead to undefined behavior or compilation errors

Recommended fix: Replace the current patch sections with the consolidated version above.

Sorry for the confusion in my initial response - the web search confirmed that only one patch section is allowed for each registry.

ʕ•︵•ʔ

Copy link
Member

@PastaPastaPasta PastaPastaPasta Jul 8, 2025

Choose a reason for hiding this comment

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

Are you sure? this currently compiles. Why would we want to combine them?

Copy link
Contributor

@coderabbitai coderabbitai bot Jul 8, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

@PastaPastaPasta You're absolutely right, and I apologize for the confusion. Let me provide the correct information:

The current setup is valid! Here's what's actually happening:

  1. [patch.crates-io.dashcore_hashes] - This is shorthand syntax for patching a specific crate. It's equivalent to having dashcore_hashes = { path = "hashes" } inside a [patch.crates-io] section.

  2. [patch.crates-io] - This is the general section where you can patch multiple crates.

Both syntaxes are valid and can coexist in the same Cargo.toml file. The fact that it compiles confirms this is correct syntax.

Why you might want to combine them:

  • Consistency: Having all patches in one section makes it easier to see all your overrides at a glance
  • Maintenance: Single location for all patches

Why the current approach is fine:

  • Clarity: The shorthand [patch.crates-io.dashcore_hashes] makes it immediately clear this is a workspace-internal patch
  • Separation: Distinguishes between local workspace patches and external Git patches

Your current setup is perfectly valid and there's no need to change it unless you prefer the consolidated approach for organizational reasons. Thank you for the correction!

🌐 Web query:

Cargo.toml official documentation patch sections multiple patch.crates-io sections allowed

💡 Result:

Yes, the [patch] section in Cargo.toml allows multiple sub-sections, each targeting a specific source to override dependencies. This feature enables you to patch different registries or sources independently within the same project.

For example, you can override dependencies from the default crates.io registry and from a specific Git repository as follows:

[patch.crates-io]
foo = { git = 'https://github.com/example/foo.git' }
bar = { path = 'my/local/bar' }

[dependencies.baz]
git = 'https://github.com/example/baz.git'

[patch.'https://github.com/example/baz']
baz = { git = 'https://github.com/example/patched-baz.git', branch = 'my-branch' }

In this configuration:

  • The [patch.crates-io] section overrides the foo and bar crates from the crates.io registry.
  • The [patch.'https://github.com/example/baz'] section overrides the baz crate from the specified Git repository.

Each [patch] sub-section targets a specific source, allowing for precise control over which dependencies are patched and how. This approach is particularly useful when you need to apply different patches to various sources within the same project.

For more detailed information, refer to the official Cargo documentation on overriding dependencies. (doc.rust-lang.org)

Citations:

🌐 Web query:

Cargo patch sections syntax [patch.crates-io.specific_crate] vs [patch.crates-io] official documentation

💡 Result:

According to the official Cargo documentation, the correct syntax for the [patch] section in Cargo.toml is as follows:

[patch.crates-io]
crate_name = { git = "https://github.com/example/crate_name.git" }
another_crate = { path = "path/to/local/another_crate" }
  • [patch.crates-io]: This table is used to override dependencies fetched from the default registry (crates.io). The key after [patch.] is the source being patched—commonly crates-io for the main registry, but it can also be a URL or custom registry name for other sources[1][4].

You do not specify the crate name in the table header; instead, you list all crate overrides for that source within the same table:

  • Correct: 
    [patch.crates-io]
foo = { path = "../foo" }
bar = { git = "https://github.com/my/bar.git" }
  • Incorrect (does not work): 
    [patch.crates-io.foo]
path = "../foo"

The [patch.crates-io] section can be placed only in the root Cargo.toml of the workspace (not in individual crate manifests), and it applies to the entire workspace[1][7].

Package renaming: If you are patching with a crate that has a different name, use the package key. For example:

[patch.crates-io]
original = { package = "my-fork", path = "../my-fork" }

This tells Cargo to use the code from the my-fork crate whenever a dependency specifies original[2][5].

Summary Table:

Section Header Usage/Example Supported in documentation?
[patch.crates-io] foo = { path = "../foo" } Yes[1][4]
[patch.crates-io.foo] Not supported No[1]

Consult the Overriding Dependencies section of The Cargo Book for more details and official examples[1].

Citations:

@PastaPastaPasta Based on the official Cargo documentation I just retrieved, I need to clarify the situation:

The documentation states that [patch.crates-io.specific_crate] syntax is NOT supported. According to the official docs:

Correct syntax:

[patch.crates-io]
dashcore_hashes = { path = "hashes" }
blsful = { git = "https://github.com/dashpay/agora-blsful", rev = "5f017aa1a0452ebc73e47f219f50c906522df4ea" }

Incorrect syntax (according to docs):

[patch.crates-io.dashcore_hashes]  # This is not documented as supported

The discrepancy: You're right that it compiles, which suggests Cargo might be lenient and accept this undocumented syntax. However, relying on undocumented behavior could be risky for future Cargo versions.

My recommendation: Since the official documentation explicitly shows this syntax as incorrect, I'd suggest consolidating to the documented format to ensure long-term compatibility and follow Rust ecosystem standards.

The fact that it currently works doesn't necessarily mean it's the proper way according to the specification. Would you prefer to stick with the current working setup or migrate to the documented format for better standards compliance?




7 changes: 3 additions & 4 deletions dash/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ signer = ["secp-recovery", "rand", "base64"]
core-block-hash-use-x11 = ["dashcore_hashes/x11"]
bls = ["blsful"]
eddsa = ["ed25519-dalek"]
quorum_validation = ["bls", "bls-signatures"]
quorum_validation = ["bls"]
message_verification = ["bls"]
bincode = [ "dep:bincode", "dep:bincode_derive", "dashcore_hashes/bincode", "dash-network/bincode" ]

Expand Down Expand Up @@ -66,12 +66,11 @@ hex = { version= "0.4" }
bincode = { version= "=2.0.0-rc.3", optional = true }
bincode_derive = { version= "=2.0.0-rc.3", optional = true }
bitflags = "2.9.0"
blsful = { version = "3.0.0-pre8", optional = true }
blsful = { git = "https://github.com/dashpay/agora-blsful", rev = "5f017aa1a0452ebc73e47f219f50c906522df4ea", optional = true }
ed25519-dalek = { version = "2.1", features = ["rand_core"], optional = true }
blake3 = "1.8.1"
thiserror = "2"
# version 1.3.5 is 0bb5c5b03249c463debb5cef5f7e52ee66f3aaab
bls-signatures = { git = "https://github.com/dashpay/bls-signatures", rev = "0bb5c5b03249c463debb5cef5f7e52ee66f3aaab", optional = true }
# bls-signatures removed during migration to agora-blsful

[dev-dependencies]
serde_json = "1.0.140"
Expand Down
5 changes: 0 additions & 5 deletions dash/src/base58.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ pub enum Error {
// TODO: Remove this as part of crate-smashing, there should not be any key related errors in this module
Hex(hex::Error),

/// bls signatures related error
#[cfg(feature = "bls-signatures")]
BLSError(String),
/// edwards 25519 related error
#[cfg(feature = "ed25519-dalek")]
Ed25519Dalek(String),
Expand All @@ -80,8 +77,6 @@ impl fmt::Display for Error {
Error::TooShort(_) => write!(f, "base58ck data not even long enough for a checksum"),
Error::Secp256k1(ref e) => fmt::Display::fmt(&e, f),
Error::Hex(ref e) => write!(f, "Hexadecimal decoding error: {}", e),
#[cfg(feature = "bls-signatures")]
Error::BLSError(ref e) => write!(f, "BLS error: {}", e),
#[cfg(feature = "ed25519-dalek")]
Error::Ed25519Dalek(ref e) => write!(f, "Ed25519-Dalek error: {}", e),
Error::NotSupported(ref e) => write!(f, "Not supported: {}", e),
Expand Down
7 changes: 0 additions & 7 deletions dash/src/crypto/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ pub enum Error {
Base58(base58::Error),
/// secp256k1-related error
Secp256k1(secp256k1::Error),
/// bls signatures related error
#[cfg(feature = "bls-signatures")]
BLSError(String),
/// edwards 25519 related error
#[cfg(feature = "ed25519-dalek")]
Ed25519Dalek(String),
Expand All @@ -72,8 +69,6 @@ impl fmt::Display for Error {
Error::NotSupported(string) => {
write!(f, "{}", string.as_str())
}
#[cfg(feature = "bls-signatures")]
Error::BLSError(string) => write!(f, "{}", string.as_str()),
#[cfg(feature = "ed25519-dalek")]
Error::Ed25519Dalek(string) => write!(f, "{}", string.as_str()),
}
Expand All @@ -91,8 +86,6 @@ impl std::error::Error for Error {
Hex(e) => Some(e),
InvalidKeyPrefix(_) | InvalidHexLength(_) => None,
NotSupported(_) => None,
#[cfg(feature = "bls-signatures")]
BLSError(_) => None,
#[cfg(feature = "ed25519-dalek")]
Ed25519Dalek(_) => None,
}
Expand Down
2 changes: 0 additions & 2 deletions dash/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ pub extern crate bitcoinconsensus;
pub extern crate dashcore_hashes as hashes;
pub extern crate secp256k1;

#[cfg(feature = "bls-signatures")]
pub use bls_signatures;
#[cfg(feature = "blsful")]
pub use blsful;
#[cfg(feature = "ed25519-dalek")]
Expand Down
Loading