Skip to content
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

[poc] DO NOT MERGE: shared account ID is off curve #4371

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions data_model/Cargo.toml
Expand Up @@ -53,6 +53,7 @@ getset = { workspace = true }
strum = { workspace = true, features = ["derive"] }
base64 = { workspace = true, features = ["alloc"] }
once_cell = { workspace = true, optional = true }
blake2 = { version = "0.10.6", default-features = false }

[dev-dependencies]
iroha_crypto = { workspace = true, features = ["rand"] }
Expand Down
193 changes: 193 additions & 0 deletions data_model/src/account.rs
Expand Up @@ -560,3 +560,196 @@ mod tests {
}
}
}

/// Proof of concept for the new account, with non-essential parts omitted. There is room for optimization in actual implementation.
#[allow(dead_code)]
mod poc {
use blake2::{digest::consts::U32, Blake2b, Digest};
use iroha_crypto::Algorithm;

use super::{PublicKey, Signatories};

/// Opaque bytes interpreted to either personal or shared account ID.
type RawId = [u8; 32];
Copy link
Contributor

Choose a reason for hiding this comment

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

haha, really suspicious

Copy link
Contributor

Choose a reason for hiding this comment

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

can we, please, not include shared accounts in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a PR, but how multi-signatory account holds its ID will soon become important.
Since this is still intended to be a compliant proposal to the discussion so far, could you be more specific about the issue or provide an alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you just advised not to preemptively implement shared accounts in the 1st PR. If so, we are on the same page

Copy link
Contributor

Choose a reason for hiding this comment

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

If so, this PoC should be entirely re-written? I see the code is mostly about shared accounts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this PoC is a part of the specification or discussion, and is completely different from the 1st PR.
So this is expected to be closed at some point


struct AccountId(RawId);

struct Account {
/// - For personal accounts, essentially the public key itself.
/// - For shared accounts, the hash value of `credential`, which is ensured to be off curve and thus have no associated private key.
id: AccountId,
/// Identity of shared account which is also the authentication policy.
credential: Option<Credential>,
}

struct Credential {
/// Possible signatories of the shared account.
signatories: Signatories,
/// Minimum number of signatures by `signatories` required to pass authentication.
/// Replaces the previous `SignatureCheckCondition`.
threshold: u32,
/// Value to adjust the hash value of this `Credential`.
bump_seed: u8,
}

/// Unverified account creation request.
enum AccountRequest {
Personal(RawId),
Shared {
signatories: Vec<RawId>,
threshold: u32,
},
}

/// An utility for this PoC.
type Result<T> = core::result::Result<T, &'static str>;

/// An utility for this PoC. Fails if `raw_id` is off ed25519 curve.
fn valid_public_key_from(raw_id: &RawId) -> Result<PublicKey> {
PublicKey::from_bytes(Algorithm::default(), raw_id)
.map_err(|_| "failed to decode as a public key")
}

impl Account {
fn new(req: AccountRequest) -> Result<Self> {
match req {
AccountRequest::Personal(raw_id) => {
if valid_public_key_from(&raw_id).is_ok() {
Ok(Self {
id: AccountId(raw_id),
credential: None,
})
} else {
Err("invalid personal id")
}
}
AccountRequest::Shared {
signatories,
threshold,
} => {
let signatories_len = signatories.len() as u32;
if !(1 < signatories_len) {
return Err("shared accounts must have at least 2 signatories");
}
if !(0 < threshold && threshold <= signatories_len) {
return Err("threshold is out of range");
}
let Ok(signatories) = signatories
.iter()
.map(|raw_id| valid_public_key_from(raw_id))
.collect()
else {
return Err("invalid signatory");
};
let mut credential = Credential::new(signatories, threshold);
while credential.is_hashed_to_public_key() {
credential.bump()
}
Ok(Self {
id: AccountId(credential.hash()),
credential: Some(credential),
})
}
}
}

fn is_personal(&self) -> bool {
self.credential.is_none()
}

fn is_shared(&self) -> bool {
self.credential.is_some()
}
}

impl Credential {
/// Not checked if its hash can collide with any public key.
fn new(signatories: Signatories, threshold: u32) -> Self {
Self {
signatories,
threshold,
bump_seed: 0,
}
}

fn is_hashed_to_public_key(&self) -> bool {
valid_public_key_from(&self.hash()).is_ok()
}

/// # Panics
///
/// Panics in the statistically improbable event that a successful bump seed could not be found.
fn bump(&mut self) {
self.bump_seed += 1;
}

#[allow(unsafe_code)]
fn hash(&self) -> RawId {
let mut hasher = Blake2b::<U32>::new();
for signatory in &self.signatories {
hasher.update(signatory.to_bytes().1)
}
// SAFETY: Src and Dst of transmute have the same representation.
let threshold = unsafe { std::mem::transmute::<u32, [u8; 4]>(self.threshold) };
hasher.update(&threshold);
hasher.update(&[self.bump_seed]);
hasher.finalize().into()
}
}

#[cfg(test)]
mod tests {
use iroha_crypto::KeyPair;

use super::*;

#[test]
fn spec() {
let gen_valid_personal_id = || {
KeyPair::random()
.public_key()
.to_bytes()
.1
.try_into()
.expect("32 bytes")
};

let req_personal = AccountRequest::Personal(gen_valid_personal_id());
let req_shared = AccountRequest::Shared {
signatories: (0..3).map(|_| gen_valid_personal_id()).collect(),
threshold: 2,
};

let account_personal = Account::new(req_personal).expect("valid request");
let account_shared = Account::new(req_shared).expect("valid request");

assert!(account_personal.is_personal());
assert!(account_shared.is_shared());

assert!(valid_public_key_from(&account_personal.id.0).is_ok());
assert!(valid_public_key_from(&account_shared.id.0).is_err());

let signatories_len = account_shared
.credential
.as_ref()
.expect("shared accounts should have")
.signatories
.len() as u32;
assert!(1 < signatories_len);
let threshold = account_shared
.credential
.as_ref()
.expect("shared accounts should have")
.threshold;
assert!(0 < threshold && threshold <= signatories_len);

assert_eq!(
account_shared.id.0,
account_shared
.credential
.expect("shared accounts should have")
.hash()
);
}
}
}