-
Notifications
You must be signed in to change notification settings - Fork 112
refactor(gring): Replaced gring and ethexe-signer with universal gsigner crate #4909
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
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # ethexe/consensus/src/mock.rs # ethexe/consensus/src/validator/coordinator.rs # ethexe/network/src/lib.rs # ethexe/service/src/lib.rs
Resolved conflicts: - ethexe/consensus: Updated to use ValidatorMessage wrapper for signing - ethexe/network: Added nonempty, auto_impl, lru dependencies - ethexe/observer/tests: Use gsigner with nonempty - ethexe/service/tests: Added NonEmpty import - gsigner/signature: Merged codec features and Hash impl
# Conflicts: # Cargo.lock # ethexe/common/src/tx_pool.rs # ethexe/consensus/src/utils.rs # ethexe/consensus/src/validator/coordinator.rs # ethexe/consensus/src/validator/core.rs # ethexe/consensus/src/validator/mod.rs # ethexe/consensus/src/validator/producer.rs # ethexe/ethereum/src/deploy.rs # ethexe/ethereum/src/lib.rs # ethexe/network/src/validator.rs # ethexe/observer/src/tests.rs # ethexe/service/src/lib.rs # ethexe/service/src/tests/mod.rs # ethexe/service/src/tests/utils/env.rs # gsigner/src/schemes/secp256k1/address.rs
gsigner/src/cli/commands.rs
Outdated
| pub enum Secp256k1Commands { | ||
| #[command(about = "Generate a new secp256k1 keypair")] | ||
| Generate { | ||
| #[arg(short, long, help = "Storage directory (default: memory only)")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need some default storage on disk for cli as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now by default it will be written to the disk, and in memory it will live on the additional argument memory
gsigner/src/cli/commands.rs
Outdated
|
|
||
| /// Keyring subcommands | ||
| #[derive(Subcommand, Debug, Clone)] | ||
| pub enum KeyringCommands { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats keyring here? isn't all of this is keyring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some commands aren't related to keyring storage implementation, so they live at the root scheme level
ethexe/common/src/gear.rs
Outdated
| let mut bytes = Vec::new(); | ||
| for claim in value_claims { | ||
| bytes.extend_from_slice(claim.message_id.into_bytes().as_ref()); | ||
| bytes.extend_from_slice(claim.destination.to_address_lossy().as_ref()); | ||
| bytes.extend_from_slice(&claim.value.to_be_bytes()); | ||
| } | ||
| sha3::Keccak256::digest(&bytes).into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was exactly old implementation for [ValueClaim], so you only need value_claims.to_digest().as_ref().update_hasher(hasher)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, there WAS implementation for [ValueClaim], but now trait ToDigit live in gsigner crate and so I can't implement ToDigit for arrays
ethexe/common/src/gear.rs
Outdated
| let mut message_hashes = Vec::with_capacity(messages.len() * 32); | ||
| for message in messages { | ||
| message_hashes.extend_from_slice(message.to_digest().as_ref()); | ||
| } | ||
| sha3::Keccak256::digest(&message_hashes).into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use sha3::Keccak256 directly to feed it with message digest bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasher.update(messages.to_digest()); is enough I think
gsigner/src/signer.rs
Outdated
| } | ||
|
|
||
| /// Get write access to the underlying storage. | ||
| pub fn storage_mut(&self) -> RwLockWriteGuard<'_, dyn KeyStorage<S>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it's decided to use RwLock? Is signer used in multi-threaded env somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Signer type is Clone and wraps the keyring in Arc<RwLock<…>>, making it Send + Sync. That’s what lets the network/service code (e.g., ethexe service, network validators, and async tasks) share a signer across threads/tasks safely
The overhead is minimal for our use (mostly single-threaded CLI), our operations are coarse (IO, serde, crypto), so the lock cost is noise
gsigner/src/storage/fs.rs
Outdated
| } | ||
|
|
||
| /// Create temporary filesystem storage. | ||
| pub fn tmp() -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it never actually needed because of memory storage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used in tests
# Conflicts: # Cargo.lock # ethexe/common/src/gear.rs # ethexe/common/src/tx_pool.rs # ethexe/db/src/database.rs # ethexe/service/src/lib.rs # ethexe/service/src/tests/mod.rs # ethexe/service/src/tests/utils/env.rs # ethexe/tx-pool/src/tests.rs # gsdk/Cargo.toml # gsdk/src/signer/mod.rs # gsigner/src/schemes/secp256k1/address.rs # gsigner/src/schemes/secp256k1/digest.rs # gsigner/src/schemes/secp256k1/signature.rs
# Conflicts: # Cargo.lock # ethexe/cli/src/commands/tx.rs # ethexe/consensus/src/utils.rs # ethexe/consensus/src/validator/core.rs # ethexe/consensus/src/validator/participant.rs # ethexe/consensus/src/validator/producer.rs # ethexe/service/src/lib.rs # ethexe/service/src/tests/utils/env.rs
# Conflicts: # Cargo.lock # ethexe/common/src/injected.rs # ethexe/consensus/src/validator/coordinator.rs # ethexe/consensus/src/validator/core.rs # ethexe/consensus/src/validator/mod.rs # ethexe/consensus/src/validator/tx_pool.rs # ethexe/ethereum/src/lib.rs # ethexe/service/src/lib.rs
# Conflicts: # Cargo.lock # ethexe/network/Cargo.toml
# Conflicts: # gsdk/src/signer/mod.rs
# Conflicts: # ethexe/service/src/lib.rs
# Conflicts: # ethexe/ethereum/src/lib.rs # ethexe/ethereum/src/router/mod.rs
# Conflicts: # Cargo.lock # ethexe/network/src/lib.rs # ethexe/network/src/validator/topic.rs
# Conflicts: # ethexe/network/src/validator/topic.rs
# Conflicts: # ethexe/cli/src/commands/key.rs # ethexe/common/src/crypto/keys.rs # ethexe/service/src/lib.rs
# Conflicts: # Cargo.lock # ethexe/signer/Cargo.toml # gcli/Cargo.toml # gcli/src/app.rs # gcli/src/lib.rs # gsdk/Cargo.toml # gsdk/src/signer/mod.rs # utils/gring/Cargo.toml
# Conflicts: # Cargo.lock
# Conflicts: # Cargo.lock # ethexe/cli/Cargo.toml # ethexe/cli/src/commands/tx.rs # ethexe/signer/src/signer.rs # gsigner/src/schemes/secp256k1/address.rs # gsigner/src/schemes/secp256k1/signature.rs
gsigner Feature Comparison
This document compares the features implemented in
gsignerwith the legacy cratesethexe-signer(secp256k1-focused Ethereum signer)gring(utils/gring, sr25519-only keyring/CLI compatible with polkadot-js keystores)Feature Matrix
peer-idfeature)Detailed Feature Descriptions
Secp256k1 Features (from ethexe-signer)
✅ Implemented
sp_core::ecdsa::PairAPI Comparison
ethexe-signer:
gsigner:
Sr25519 Features (from gring)
✅ Implemented
API Comparison
gring:
gsigner:
Ed25519 Features
✅ Implemented
sp_core::ed25519::Pairfor derivations matching Substrate toolingSigner<Ed25519>with filesystem and memory storageNew Features in gsigner
Unified Interface
Signer<S>type works with any schemeExtension Traits
Universal Address Type
Addressenum supports both Ethereum and Substrate addressesEnhanced Type Safety
Improved Error Handling
anyhow::Resultreturn typesProduction Parity Updates
sp_corepair/public/signature types (secp256k1, ed25519, sr25519), so SURIs, SS58 addresses, and SCALE codecs match upstream Substrate tooling byte-for-byte.sp_core::ecdsa::Signaturevalues.Storage Abstraction
StorageLocationArgsunifies the CLI surface for every scheme, providing--path/--storage,--memory, and--storage-passwordflags.$XDG_DATA_HOME/gsigner/<scheme>on Unix-like systems).Signer::fsandSigner::fs_with_passwordreturnResult, surfacing IO and decoding failures instead of panicking.peer-idfeature for secp256k1 and ed25519 keys.--format human|plain|json, making scripting easier while keeping human-friendly defaults.Legacy crates
utils/gring): sr25519-only; CLI supports add/import, generate (with optional vanity), list/use primary, sign/verify with a fixed signing context; keystore path is fixed to the platform data dir (e.g.~/Library/Application Support/gringon macOS) and uses polkadot-js JSON with scrypt + xsalsa20 encryption; no multi-scheme support, no output-format selection, no peer-id; only sr25519 keys and PKCS8+SURI flows.Migration Guide
From ethexe-signer
All ethexe-signer functionality is preserved with minimal API changes.
From gring
Keyring APIs remain largely unchanged across schemes and share common primary-key behaviour.