Skip to content

feat(signer): Request wallets instead of accounts in Dirk setup #303

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 44 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Build Signer Docker Image

on:
push:
branches:
- "mb/remove_accounts"

permissions:
contents: read
packages: write

jobs:
build-and-push-signer-docker:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: 0
submodules: true

- name: Set up QEMU
uses: docker/setup-qemu-action@v3

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

- name: Login to GitHub Container Registry
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Build and push signer Docker image
uses: docker/build-push-action@v6
with:
context: .
push: true
platforms: linux/amd64,linux/arm64
tags: ghcr.io/commit-boost/signer:dirk-easier-config
cache-from: type=registry,ref=ghcr.io/commit-boost/signer:buildcache
cache-to: type=registry,ref=ghcr.io/commit-boost/signer:buildcache,mode=max
file: provisioning/signer.Dockerfile
6 changes: 3 additions & 3 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,13 @@ url = "http://0xa119589bb33ef52acbb8116832bec2b58fca590fe5c85eac5d3230b44d5bc09f
# server_name = "localhost-1"
# Complete URL of a Dirk gateway
# url = "https://localhost:8881"
# Accounts to use as consensus keys
# accounts = ["Wallet1/Account1", "DistributedWallet/Account1"]
# Wallets to load consensus keys from
# accounts = ["Wallet1", "DistributedWallet"]

# [[signer.dirk.hosts]]
# server_name = "localhost-2"
# url = "https://localhost:8882"
# accounts = ["Wallet2/Account2", "DistributedWallet/Account1"]
# accounts = ["Wallet2", "DistributedWallet"]

# Configuration for how the Signer module should store proxy delegations.
# OPTIONAL
Expand Down
4 changes: 2 additions & 2 deletions crates/common/src/config/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ pub struct DirkHostConfig {
pub server_name: Option<String>,
/// Complete URL of the Dirk server
pub url: Url,
/// Accounts used as consensus keys
pub accounts: Vec<String>,
/// Wallets to load consensus keys from
pub wallets: Vec<String>,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
Expand Down
94 changes: 26 additions & 68 deletions crates/signer/src/manager/dirk.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
use std::{
collections::{HashMap, HashSet},
io::Write,
path::PathBuf,
};
use std::{collections::HashMap, io::Write, path::PathBuf};

use alloy::{hex, rpc::types::beacon::constants::BLS_SIGNATURE_BYTES_LEN};
use blsful::inner_types::{Field, G2Affine, G2Projective, Group, Scalar};
Expand Down Expand Up @@ -37,10 +33,10 @@ enum Account {
}

impl Account {
pub fn full_name(&self) -> String {
pub fn name(&self) -> &str {
match self {
Account::Simple(account) => format!("{}/{}", account.wallet, account.name),
Account::Distributed(account) => format!("{}/{}", account.wallet, account.name),
Account::Simple(account) => &account.name,
Account::Distributed(account) => &account.name,
}
}
}
Expand All @@ -49,7 +45,6 @@ impl Account {
struct SimpleAccount {
public_key: BlsPublicKey,
connection: Channel,
wallet: String,
name: String,
}

Expand All @@ -58,7 +53,6 @@ struct DistributedAccount {
composite_public_key: BlsPublicKey,
participants: HashMap<u32, Channel>,
threshold: u32,
wallet: String,
name: String,
}

Expand Down Expand Up @@ -107,14 +101,8 @@ impl DirkManager {
}
};

let wallets: HashSet<String> = host
.accounts
.iter()
.map(|account| decompose_name(account).unwrap_or_default().0)
.collect();

let accounts_response = match ListerClient::new(channel.clone())
.list_accounts(ListAccountsRequest { paths: wallets.into_iter().collect() })
.list_accounts(ListAccountsRequest { paths: host.wallets.clone() })
.await
{
Ok(res) => res,
Expand All @@ -130,12 +118,7 @@ impl DirkManager {
}

let accounts_response = accounts_response.into_inner();
load_simple_accounts(
accounts_response.accounts,
&host,
&channel,
&mut consensus_accounts,
);
load_simple_accounts(accounts_response.accounts, &channel, &mut consensus_accounts);
load_distributed_accounts(
accounts_response.distributed_accounts,
&host,
Expand All @@ -144,15 +127,6 @@ impl DirkManager {
)
.map_err(|error| warn!("{error}"))
.ok();

for account in host.accounts {
if !consensus_accounts
.values()
.any(|account| account.full_name() == account.full_name())
{
warn!("Account {account} not found in server {}", host.url);
}
}
}

debug!(
Expand Down Expand Up @@ -293,10 +267,7 @@ impl DirkManager {
.sign(SignRequest {
data: object_root.to_vec(),
domain: compute_domain(self.chain, COMMIT_BOOST_DOMAIN).to_vec(),
id: Some(sign_request::Id::Account(format!(
"{}/{}",
account.wallet, account.name
))),
id: Some(sign_request::Id::Account(account.name.clone())),
})
.map(|res| (res, *id))
.await
Expand Down Expand Up @@ -392,7 +363,7 @@ impl DirkManager {

let response = AccountManagerClient::new(consensus.connection.clone())
.generate(GenerateRequest {
account: format!("{}/{}/{module}/{uuid}", consensus.wallet, consensus.name),
account: format!("{}/{module}/{uuid}", consensus.name),
passphrase: password.as_bytes().to_vec(),
participants: 1,
signing_threshold: 1,
Expand All @@ -418,7 +389,6 @@ impl DirkManager {
inner: Account::Simple(SimpleAccount {
public_key: proxy_key,
connection: consensus.connection.clone(),
wallet: consensus.wallet.clone(),
name: format!("{}/{module}/{uuid}", consensus.name),
}),
};
Expand Down Expand Up @@ -450,7 +420,7 @@ impl DirkManager {
for (id, channel) in consensus.participants.iter() {
let Ok(response) = AccountManagerClient::new(channel.clone())
.generate(GenerateRequest {
account: format!("{}/{}/{module}/{uuid}", consensus.wallet, consensus.name),
account: format!("{}/{module}/{uuid}", consensus.name),
passphrase: password.as_bytes().to_vec(),
participants: consensus.participants.len() as u32,
signing_threshold: consensus.threshold,
Expand Down Expand Up @@ -479,7 +449,6 @@ impl DirkManager {
composite_public_key: proxy_key,
participants: consensus.participants.clone(),
threshold: consensus.threshold,
wallet: consensus.wallet.clone(),
name: format!("{}/{module}/{uuid}", consensus.name),
}),
};
Expand All @@ -506,8 +475,8 @@ impl DirkManager {

/// Store the password for a proxy account in disk
fn store_password(&self, account: &ProxyAccount, password: String) -> eyre::Result<()> {
let full_name = account.inner.full_name();
let (parent, name) = full_name.rsplit_once('/').ok_or_eyre("Invalid account name")?;
let name = account.inner.name();
let (parent, name) = name.rsplit_once('/').ok_or_eyre("Invalid account name")?;
let parent_path = self.secrets_path.join(parent);

std::fs::create_dir_all(parent_path.clone())?;
Expand All @@ -530,7 +499,7 @@ impl DirkManager {
let request = async move {
let response = AccountManagerClient::new(channel.clone())
.unlock(UnlockAccountRequest {
account: account.full_name(),
account: account.name().to_string(),
passphrase: password.as_bytes().to_vec(),
})
.await;
Expand Down Expand Up @@ -584,40 +553,26 @@ async fn connect(
.map_err(eyre::Error::from)
}

/// Decompose a full account name into wallet and name
fn decompose_name(full_name: &str) -> eyre::Result<(String, String)> {
full_name
.split_once('/')
.map(|(wallet, name)| (wallet.to_string(), name.to_string()))
.ok_or_else(|| eyre::eyre!("Invalid account name"))
}

/// Load `SimpleAccount`s into the consensus accounts map
fn load_simple_accounts(
accounts: Vec<crate::proto::v1::Account>,
host: &DirkHostConfig,
channel: &Channel,
consensus_accounts: &mut HashMap<BlsPublicKey, Account>,
) {
for account in accounts {
if !host.accounts.contains(&account.name) {
if name_matches_proxy(&account.name) {
debug!(account = account.name, "Ignoring account assuming it's a proxy key");
continue;
}

let Ok((wallet, name)) = decompose_name(&account.name) else {
warn!("Invalid account name {}", account.name);
continue;
};

match BlsPublicKey::try_from(account.public_key.as_slice()) {
Ok(public_key) => {
consensus_accounts.insert(
public_key,
Account::Simple(SimpleAccount {
public_key,
connection: channel.clone(),
wallet,
name,
name: account.name,
}),
);
}
Expand All @@ -643,7 +598,8 @@ fn load_distributed_accounts(
.ok_or(eyre::eyre!("Host name not found for server {}", host.url))?;

for account in accounts {
if !host.accounts.contains(&account.name) {
if name_matches_proxy(&account.name) {
debug!(account = account.name, "Ignoring account assuming it's a proxy key");
continue;
}

Expand All @@ -664,11 +620,6 @@ fn load_distributed_accounts(
participants.insert(participant_id as u32, channel.clone());
}
None => {
let Ok((wallet, name)) = decompose_name(&account.name) else {
warn!("Invalid account name {}", account.name);
continue;
};

let mut participants = HashMap::with_capacity(account.participants.len());
participants.insert(participant_id as u32, channel.clone());

Expand All @@ -678,8 +629,7 @@ fn load_distributed_accounts(
composite_public_key: public_key,
participants,
threshold: account.signing_threshold,
wallet,
name,
name: account.name,
}),
);
}
Expand Down Expand Up @@ -739,6 +689,14 @@ fn random_password() -> String {
hex::encode(password_bytes)
}

/// Returns whether the name of an account has a proxy name format.
///
/// i.e., `{wallet}/{consensus_proxy}/{module}/{uuid}`
fn name_matches_proxy(name: &str) -> bool {
name.split("/").count() > 3 &&
name.rsplit_once("/").is_some_and(|(_, name)| uuid::Uuid::parse_str(name).is_ok())
}

mod test {

#[test]
Expand Down
6 changes: 3 additions & 3 deletions docs/docs/get_started/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,16 @@ ca_cert_path = "/path/to/ca.crt"
[[signer.dirk.hosts]]
server_name = "localhost-1"
url = "https://localhost-1:8081"
accounts = ["SomeWallet/SomeAccount", "DistributedWallet/Account1"]
wallets = ["SomeWallet", "DistributedWallet"]

[[signer.dirk.hosts]]
server_name = "localhost-2"
url = "https://localhost-2:8082"
accounts = ["AnotherWallet/AnotherAccount", "DistributedWallet/Account1"]
wallets = ["AnotherWallet", "DistributedWallet"]
```

- `cert_path` and `key_path` are the paths to the client certificate and key used to authenticate with Dirk.
- `accounts` is a list of accounts that the Signer module will consider as the consensus keys. Each account has the format `<WALLET_NAME>/<ACCOUNT>`. Accounts can be from different wallets. Generated proxy keys will have format `<WALLET_NAME>/<ACCOUNT>/<MODULE_ID>/<UUID>`.
- `wallets` is a list of wallets from which the Signer module will load all accounts as consensus keys. Generated proxy keys will have format `<WALLET_NAME>/<ACCOUNT>/<MODULE_ID>/<UUID>`, so accounts found with that pattern will be ignored.
- `secrets_path` is the path to the folder containing the passwords of the generated proxy accounts, which will be stored in `<secrets_path>/<WALLET_NAME>/<ACCOUNT>/<MODULE_ID>/<UUID>.pass`.

Additionally, you can set a proxy store so that the delegation signatures for generated proxy keys are stored locally. As these signatures are not sensitive, the only supported store type is `File`:
Expand Down
2 changes: 1 addition & 1 deletion examples/configs/dirk_signer.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ ca_cert_path = "/path/to/ca.crt"
# Example of a single Dirk host
[[signer.dirk.hosts]]
url = "https://gateway.dirk.url"
accounts = ["wallet1/accountX", "wallet2/accountY"]
wallets = ["wallet1", "wallet2"]

[signer.dirk.store]
proxy_dir = "/path/to/proxies"
Expand Down
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ docker-build-test-modules:
docker build -t test_status_api . -f examples/status_api/Dockerfile

test:
cargo test --all-features
cargo test --all-features
Loading