WIP: feat-shamir (old) #103
Reference in New Issue
Block a user
Delete Branch "feat-shamir"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
DO NOT MERGE!
Separate database interactions in
ProposalManagerinto separate DBAL trait, so we could mock it in the future usingautomockcrate and test properly.ProposalManageris also too bloated: not only does it coordinate voting, it also tried to execute the result of votes. Which is not its responsibility. Instead, it should broadcast which votes succeed onMessageBusand other relevant actors should execute it instead.@@ -8,6 +8,7 @@ use rand::RngExt;pub static CLIENT_CONTEXT: &[u8] = b"arbiter_client";pub static OPERATOR_CONTEXT: &[u8] = b"arbiter_operator";pub static GOVERNANCE_CONTEXT: &[u8] = b"arbiter_governance_vote";declare enum here with 3 possible context, associate a binary data with them:
https://docs.rs/strum/latest/strum/derive.IntoStaticStr.html
@@ -92,1 +93,4 @@}#[must_use]pub fn verify_message(&self, message: &[u8], context: &[u8], signature: &Signature) -> bool {here accept enum from previous comment instead of random byte slice
@@ -56,6 +56,7 @@ create table if not exists operator (share blob not null,share_nonce blob not null,share_salt blob not null default (randomblob(32)),there should be no default and salt should always be supplied by our daemon, wigga
@@ -218,0 +220,4 @@create table if not exists proposal (id integer not null primary key,kind text not null,payload blob not null,why the fuck do you store binary data inside relational database. declare tables, wigga
@@ -218,0 +224,4 @@initiator_id integer not null references operator_identity(id) on delete restrict,created_at integer not null default(unixepoch('now')),expires_at integer not null,status text not null default 'pending'expires_atalready implied when the proposal expires, so removeexpiredfrom a list of possible values, wigga@@ -218,0 +241,4 @@create table if not exists proposal_result (proposal_id integer not null primary key references proposal(id) on delete cascade,data blob not null,why the fuck do you store binary data inside relational database. declare tables, wigga
@@ -0,0 +35,4 @@}#[derive(Debug, Clone)]pub enum ProposalKind {ProposalKindTagshould be derived using this enum andstrumcrate, wigga@@ -0,0 +43,4 @@wallet_id: i32,client_id: i32,},ApproveServerUpdate,Scope creep, remove this for now
@@ -0,0 +48,4 @@old_operator_id: i32,new_pubkey: Vec<u8>,},UpdateShamirParameters {Explain what is the
UpdateShamirParameters. Do you want to edit the quorum after committee has already formed??? wiggaAfter some effort I got a very interesting answer. It was used as a trigger to start rekeying.
Replaced with a more suitable name and removed body.
@@ -0,0 +61,4 @@impl ProposalKind {pub const fn tag(&self) -> ProposalKindTag {match self {explained alr
@@ -0,0 +76,4 @@self.tag().into()}pub fn encode_payload(&self) -> Vec<u8> {Wigga, please. Remove homebrew protocol, instead, make those as protobuf entities. And convert to them using already established traits, wigga
@@ -0,0 +174,4 @@#[derive(Debug, Clone, PartialEq, Eq)]pub enum VoteOutcome {Pending,QuorumApproved,remove
Quromprefix from the variants@@ -0,0 +195,4 @@#[error("Database query error: {0}")]DatabaseQuery(#[from] diesel::result::Error),#[error("Execution failed: {0}")]ExecutionFailed(String),ProposalManagershould focus on one thing, and one thing only, the vote coordination: not the outcome execution.Otherwise, this actor becomes too bloated, like your code currently is. Wigga.
@@ -0,0 +209,4 @@#[derive(Debug)]pub struct ProposalSummary {pub id: i32,pub kind: String,Vote kind should be a enum. Not a string. We are here forcing type-safety, only for you, wigga, to break it with your strings. wigga.
@@ -0,0 +219,4 @@pub struct ProposalManager {pub(crate) db: db::DatabasePool,pub(crate) vault: ActorRef<Vault>,pub(crate) evm: ActorRef<EvmActor>,ProposalManagershould focus on one thing, and one thing only, the vote coordination: not the outcome execution.Otherwise, this actor becomes too bloated, like your code currently is. Wigga.
@@ -0,0 +271,4 @@) -> Result<i32, Error> {let ttl = ttl_secs.unwrap_or(DEFAULT_TTL_SECS);let expires_at = SqliteTimestamp::from(Utc::now() + chrono::Duration::seconds(ttl));add maximum
ttlcheck here@@ -0,0 +290,4 @@}#[message]pub async fn query_pending(&mut self, operator_id: i32) -> Vec<ProposalSummary> {let's use
OperatorIdunit type here@@ -0,0 +303,4 @@return vec![];};let voted_ids: Vec<i32> = schema::proposal_vote::tableunit type for ids here as well
@@ -0,0 +320,4 @@let mut summaries = Vec::with_capacity(proposals.len());for p in proposals {let approve_count: i64 = schema::proposal_vote::table2n+1 query problem here, replace those queries with aggregate ones
@@ -0,0 +392,4 @@})?;// Check for duplicate vote before status check so AlreadyVoted takes prioritylet existing: i64 = schema::proposal_vote::tablethere is
exists()function in diesel, which returnstrue/falseif record exist@@ -0,0 +421,4 @@.map_err(|()| Error::InvalidSignature)?;// Canonical vote message: proposal_id (i64 big-endian) || approve (u8)let mut vote_msg = Vec::with_capacity(9);extract this signature concatenation into either a separate function, or better into
cryptosubmodule.@@ -0,0 +531,4 @@/// §3.6: Any ordinary operator may request recovery wake-up./// Fails if a wake-up is already pending or active.#[message]pub async fn request_recovery_wakeup(&mut self, operator_id: i32) -> Result<(), Error> {OperatorIdunit type@@ -0,0 +548,4 @@/// §3.6: Any ordinary operator may cancel a pending wake-up request./// Fails if there is no uncancelled request.#[message]pub async fn cancel_recovery_wakeup(&mut self, operator_id: i32) -> Result<(), Error> {OperatorIdunit type@@ -0,0 +609,4 @@return Err(Error::ProposalNotPending);}let pubkey_bytes: Vec<u8> = schema::recovery_operator_identity::tablesignature check is quite complex, so separate it into several helpers and put them into
cryptosubmodule@@ -0,0 +715,4 @@schema::recovery_wakeup_request::requested_at.le(diesel::dsl::sql::<diesel::sql_types::Integer,>(&format!("unixepoch('now') - {}",https://docs.rs/diesel/latest/diesel/prelude/macro.define_sql_function.html
@@ -0,0 +727,4 @@/// Returns true when there is any uncancelled wakeup request (pending or active).async fn has_uncancelled_wakeup(conn: &mut db::DatabaseConnection) -> Result<bool, Error> {let count: i64 = schema::recovery_wakeup_request::tablethere is
exists()function indiesel@@ -0,0 +735,4 @@Ok(count > 0)}async fn execute_proposal(&self, proposal: &Proposal) -> Result<(), Error> {remove this, separation of concerns
JIT, wigga
- Add `rekey.proto` with `ContributePassphrase` / `ContributeRecoveryPassphrase` / `RekeyResult` - Wire `rekey` as a 4th vault stream payload in `vault.proto` and gRPC dispatch - Add `RekeyRootKey` message to `Vault` actor: generates new random seal key, re-encrypts root key, writes new `root_key_history` row - Add `StartRekey`, `ContributeRekey`, `ContributeRecoveryRekey` messages to `VaultCoordinator`; `finalize_rekey` uses threshold-1 fast path identical to bootstrap - `execute_replace_operator` now UPDATEs `operator_identity.public_key` in-place (avoids FK constraint violation), deletes stale `operator` share row, then triggers `StartRekey` - `execute_update_shamir_parameters` triggers `StartRekey` instead of warning stub - `ProposalKind::ReplaceOperator` carries `old_operator_id`; encode/decode updated accordingly - `GlobalActors::spawn` extracts `vault_coordinator` before `Ok(Self { … })` so it can be cloned into `ProposalManager::new` - Add `handle_rekey` in session handlers forwarding passphrase contributions to `VaultCoordinator` - Fix test: rename `replace_operator_inserts_identity_row` → `replace_operator_updates_pubkey_and_starts_rekey`, assert count stays 1 and pubkey is updatedSigningContextenum a501283b0cd8dd17ee92toa80cd39695feat-shamirto WIP: feat-shamir (old)View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.