feat(vault)!: add multi-operator Shamir custody #106

Open
CleverWild wants to merge 2 commits from feat-shamir-custody into feat-shamir-v2
Member
No description provided.
CleverWild added 1 commit 2026-09-10 19:47:56 +00:00
CleverWild added a new dependency 2026-09-10 19:50:38 +00:00
CleverWild force-pushed feat-shamir-custody from 666cf52755 to c722712166 2026-09-11 09:13:33 +00:00 Compare
Skipper requested changes 2026-09-11 11:39:40 +00:00
@@ -78,6 +78,7 @@ pub_underscore_fields = "allow"
redundant_pub_crate = "allow"
uninhabited_references = "allow" # safe with unsafe_code = "forbid" and standard uninhabited pattern (match *self {})
too-many-lines = "allow" # this is a very common pattern in server code, and it's not always possible to break it down into smaller modules without hurting readability
unused_async_trait_impl = "allow" # to pedantic
Owner

*too

*too
CleverWild marked this conversation as resolved
@@ -90,0 +116,4 @@
});
}
let registered = diesel::select(diesel::dsl::exists(
Owner

use diesel::prelude

use `diesel::prelude`
Author
Member

me completely forgot to use generate_token, wigga

me completely forgot to use `generate_token`, wigga
CleverWild marked this conversation as resolved
@@ -90,0 +123,4 @@
.await?;
let token = if registered {
match tokio::fs::read_to_string(&path).await {
Owner

no.
TOKEN SHOULD BE REGENERATED everytime

no. TOKEN SHOULD BE REGENERATED everytime
CleverWild marked this conversation as resolved
@@ -90,1 +145,4 @@
})
}
fn is_correct_token(&mut self, token: &[u8]) -> bool {
Owner

remove

remove
CleverWild marked this conversation as resolved
@@ -0,0 +40,4 @@
total: usize,
key: &mut KeyCell,
rng: impl CryptoRng,
) -> Result<SafeCell<Vec<Vec<u8>>>, ShamirError> {
Owner

Probably should be a vector of SafeCell's isntead, e.g. Vec<SafeCell<Vec<u8>>>.

Probably should be a vector of SafeCell's isntead, e.g. `Vec<SafeCell<Vec<u8>>>`.
CleverWild marked this conversation as resolved
@@ -0,0 +41,4 @@
key: &mut KeyCell,
rng: impl CryptoRng,
) -> Result<SafeCell<Vec<Vec<u8>>>, ShamirError> {
if total == 0 || threshold == 0 || threshold > total || total == 2 || total > MAX_COMMITTEE_SIZE
Owner

We do support 2 total committee members, or is that including recovery operators?
Clarify with a comment

We do support 2 total committee members, or is that including recovery operators? Clarify with a comment
Author
Member

Total is total, so we do NOT support 2 committee members without a recovery operator.
We support 1-of-1 and 2-of-3, and there is no intermediate state.

Re-check ARCHITECTURE.md, there is probably a logical contradiction.

Total is total, so we do NOT support 2 committee members without a recovery operator. We support `1-of-1` and `2-of-3`, and there is no intermediate state. Re-check `ARCHITECTURE.md`, there is probably a logical contradiction.
@@ -0,0 +54,4 @@
.try_into()
.map_err(|_| ShamirError::Split("unexpected seal key length".to_owned()))?;
if threshold == 1 {
Owner

This should be at the top of the function

This should be at the top of the function
CleverWild marked this conversation as resolved
@@ -0,0 +86,4 @@
}
let combined = shares.read_inline(|shares| {
if threshold == 1 {
Owner

Again, early exit as well

Again, early exit as well
CleverWild marked this conversation as resolved
@@ -0,0 +134,4 @@
let mut key = KeyCell::from(expected);
let rng = UnwrapErr(SysRng);
let mut shares = split_key(2, 3, &mut key, rng).expect("split should succeed");
for indexes in [[0_usize, 1_usize], [0, 2], [1, 2]] {
Owner

nice place for https://github.com/la10736/rstest

nice place for `https://github.com/la10736/rstest`
CleverWild marked this conversation as resolved
@@ -0,0 +1,162 @@
//! Storage for Shamir custody material: the reconstruction threshold and the
Owner

Purpose of this model compared to just creating an in-memory sqlite database?

Purpose of this model compared to just creating an in-memory sqlite database?
Author
Member
https://git.markettakers.org/MarketTakers/arbiter/pulls/108
CleverWild added 1 commit 2026-09-11 13:04:10 +00:00
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat-shamir-custody:feat-shamir-custody
git checkout feat-shamir-custody
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Reference: MarketTakers/arbiter#106