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

feat: ferret #202

Open
wants to merge 2 commits into
base: alpha.1
Choose a base branch
from
Open

feat: ferret #202

wants to merge 2 commits into from

Conversation

sinui0
Copy link
Collaborator

@sinui0 sinui0 commented Jan 20, 2025

This PR refactors the existing Ferret implementation to fit into the recent interface changes. I've also done some cleaning up here and there, including removing a quadratic cost algorithm present in the uniform MPCOT implementation.

Builds on #171

@sinui0 sinui0 mentioned this pull request Jan 24, 2025
Copy link
Member

@th4s th4s left a comment

Choose a reason for hiding this comment

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

🚀 looking pretty good! Some comments, mostly questions.

/// # Arguments
///
/// * `depth` - The depth of the tree.
/// * `seed` - The seed of the tree.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// * `seed` - The seed of the tree.
/// * `seed` - The seed of the tree.
/// * `leaves` - The last layer of blocks.

Comment on lines +138 to +139
/// Returns the layer at the given height.
pub fn layer(&self, height: usize) -> Option<&[Block]> {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't height the same as depth?

///
/// * `depth` - Depth of the tree.
/// * `sums` - Sum of the left or right nodes for each layer.
/// * `idx` - Index of the missing leaf.
Copy link
Member

Choose a reason for hiding this comment

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

same

Comment on lines +157 to +158
let mut left = Block::ZERO;
let mut right = Block::ZERO;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have to set both to zero here?

Comment on lines +144 to +187
static UNIFORM_PARAMS: &[LpnParameters] = &[
LpnParameters {
n: 545_656,
k: 34_643,
t: 1_050,
},
LpnParameters {
n: 1_071_888,
k: 40_800,
t: 1720,
},
LpnParameters {
n: 5_324_800,
k: 240_000,
t: 1_300,
},
LpnParameters {
n: 10_488_928,
k: 458_000,
t: 1280,
},
];

static REGULAR_PARAMS: &[LpnParameters] = &[
LpnParameters {
n: 518_656,
k: 34_643,
t: 1_013,
},
LpnParameters {
n: 1_740_800,
k: 66_400,
t: 1700,
},
LpnParameters {
n: 5_324_800,
k: 240_000,
t: 1_300,
},
LpnParameters {
n: 10_485_760,
k: 458_000,
t: 1280,
},
Copy link
Member

Choose a reason for hiding this comment

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

Are these numbers from the Ferret paper? I cannot find them.

for (item, bucket_length) in cuckoo.iter().zip(buckets.iter_buckets()) {
// pad to power of 2.
let power_of_two = (bucket_length + 1)
.checked_next_power_of_two()
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find this rounding up to the next power of 2 in the paper. On the other hand the SPCOT protocol only works with powers of 2. Is this somehow implied or how did you come up with it?


idxs.push(pos);
} else {
idxs.push(bucket_length);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be bucket_length + 1?

///
/// # Arguments
///
/// * `derandomize` - SPCOT derandomization bits from the receiver.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// * `derandomize` - SPCOT derandomization bits from the receiver.
/// * `msg` - SPCOT derandomization bits from the receiver.

///
/// # Arguments
///
/// * `derandomize` - SPCOT derandomization bits from the receiver.
Copy link
Member

Choose a reason for hiding this comment

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

same


let spcot_count: usize = spcot_lengths.iter().sum();
let masks = &self.choices[self.choices.len() - spcot_count..];
let derandomize = self.spcot.derandomize(&spcot_lengths, &spcot_idxs, masks)?;
Copy link
Member

Choose a reason for hiding this comment

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

What about truncating self.choices here, instead of line 193?

Comment on lines +129 to +132
m * ((2 * HASH_NUM as usize * params.n / m)
.checked_next_power_of_two()
.expect("The length should be less than usize::MAX / 2 - 1")
.ilog2() as usize)
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain how you arrive at this specific part?

// *Assumes the parameters are in ascending order.*
for param in params {
let cost = iteration_cost(ty, *param);
let net = param.t - cost;
Copy link
Member

Choose a reason for hiding this comment

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

Why is net = t - cost?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants