diff --git a/ipa-core/src/helpers/transport/query/oprf_shuffle.rs b/ipa-core/src/helpers/transport/query/oprf_shuffle.rs deleted file mode 100644 index e69de29bb..000000000 diff --git a/ipa-core/src/protocol/basics/mul/step.rs b/ipa-core/src/protocol/basics/mul/step.rs index 797019787..630e90965 100644 --- a/ipa-core/src/protocol/basics/mul/step.rs +++ b/ipa-core/src/protocol/basics/mul/step.rs @@ -6,12 +6,3 @@ pub(crate) enum MaliciousMultiplyStep { RandomnessForValidation, ReshareRx, } - -// This is a dummy step that is used to narrow (but never executed) the semi-honest -// context. Semi-honest implementations of `UpgradedContext::upgrade()` and subsequent -// `UpgradeToMalicious::upgrade()` narrows but these will end up in -// `UpgradedContext::upgrade_one()` or `UpgradedContext::upgrade_sparse()` which both -// return Ok() and never trigger communications. -#[derive(CompactStep)] -#[step(name = "upgrade")] -pub(crate) struct UpgradeStep; diff --git a/ipa-core/src/protocol/boolean/and.rs b/ipa-core/src/protocol/boolean/and.rs index 3ee603864..c05f5cdf8 100644 --- a/ipa-core/src/protocol/boolean/and.rs +++ b/ipa-core/src/protocol/boolean/and.rs @@ -3,12 +3,15 @@ use std::iter::zip; use crate::{ error::Error, ff::boolean::Boolean, - protocol::{basics::SecureMul, boolean::step::BoolAndStep, context::Context, RecordId}, + protocol::{ + basics::SecureMul, + boolean::{step::EightBitStep, NBitStep}, + context::Context, + RecordId, + }, secret_sharing::{replicated::semi_honest::AdditiveShare, BitDecomposed, FieldSimd}, }; -const MAX_BITS: usize = 8; - /// Matrix bitwise AND for use with vectors of bit-decomposed values. Supports up to 8 bits of input /// that is enough to support both WALR and PRF IPA use cases. /// @@ -40,14 +43,15 @@ where let b = b.into_iter(); assert_eq!(a.len(), b.len()); assert!( - a.len() <= MAX_BITS, - "Up to {MAX_BITS} values are supported, but was given a value of {len} bits", + a.len() <= usize::try_from(EightBitStep::BITS).unwrap(), + "Up to {max_bits} bit values are supported, but was given a value of {len} bits", + max_bits = EightBitStep::BITS, len = a.len() ); BitDecomposed::try_from( ctx.parallel_join(zip(a.iter(), b).enumerate().map(|(i, (a, b))| { - let ctx = ctx.narrow(&BoolAndStep::Bit(i)); + let ctx = ctx.narrow(&EightBitStep::Bit(i)); a.multiply(b, ctx, record_id) })) .await?, diff --git a/ipa-core/src/protocol/boolean/mod.rs b/ipa-core/src/protocol/boolean/mod.rs index 97d0b7382..8b80d1c27 100644 --- a/ipa-core/src/protocol/boolean/mod.rs +++ b/ipa-core/src/protocol/boolean/mod.rs @@ -17,37 +17,27 @@ pub(crate) mod step; /// /// This is a temporary solution for narrowing contexts until the infra is /// updated with a new step scheme. -pub trait BitStep: Step + From { - fn max_bit_depth() -> u32; +pub trait NBitStep: Step + From { + const BITS: u32; } -impl BitStep for EightBitStep { - fn max_bit_depth() -> u32 { - 8 - } +impl NBitStep for EightBitStep { + const BITS: u32 = 8; } -impl BitStep for SixteenBitStep { - fn max_bit_depth() -> u32 { - 16 - } +impl NBitStep for SixteenBitStep { + const BITS: u32 = 16; } -impl BitStep for ThirtyTwoBitStep { - fn max_bit_depth() -> u32 { - 32 - } +impl NBitStep for ThirtyTwoBitStep { + const BITS: u32 = 32; } -impl BitStep for TwoHundredFiftySixBitOpStep { - fn max_bit_depth() -> u32 { - 256 - } +impl NBitStep for TwoHundredFiftySixBitOpStep { + const BITS: u32 = 256; } #[cfg(test)] -impl BitStep for crate::protocol::boolean::step::DefaultBitStep { - fn max_bit_depth() -> u32 { - 256 - } +impl NBitStep for crate::protocol::boolean::step::DefaultBitStep { + const BITS: u32 = 256; } diff --git a/ipa-core/src/protocol/boolean/step.rs b/ipa-core/src/protocol/boolean/step.rs index 799f1d423..869f92726 100644 --- a/ipa-core/src/protocol/boolean/step.rs +++ b/ipa-core/src/protocol/boolean/step.rs @@ -1,11 +1,5 @@ use ipa_step_derive::CompactStep; -#[derive(CompactStep)] -pub(crate) enum BoolAndStep { - #[step(count = 8)] // keep in sync with MAX_BITS defined inside and.rs - Bit(usize), -} - #[derive(CompactStep)] pub enum EightBitStep { #[step(count = 8)] diff --git a/ipa-core/src/protocol/ipa_prf/aggregation/mod.rs b/ipa-core/src/protocol/ipa_prf/aggregation/mod.rs index 935f75344..c95c70afd 100644 --- a/ipa-core/src/protocol/ipa_prf/aggregation/mod.rs +++ b/ipa-core/src/protocol/ipa_prf/aggregation/mod.rs @@ -15,7 +15,7 @@ use crate::{ }, protocol::{ basics::{BooleanArrayMul, BooleanProtocols}, - boolean::{step::SixteenBitStep, BitStep}, + boolean::{step::SixteenBitStep, NBitStep}, context::{Context, UpgradedSemiHonestContext}, ipa_prf::{ aggregation::step::{AggregateValuesStep, AggregationStep as Step}, @@ -266,12 +266,12 @@ where let record_id = RecordId::from(i); if a.len() < usize::try_from(OV::BITS).unwrap() { assert!( - OV::BITS <= SixteenBitStep::max_bit_depth(), + OV::BITS <= SixteenBitStep::BITS, "SixteenBitStep not large enough to accomodate this sum" ); // If we have enough output bits, add and keep the carry. let (mut sum, carry) = integer_add::<_, SixteenBitStep, B>( - ctx.narrow(&AggregateValuesStep::OverflowingAdd), + ctx.narrow(&AggregateValuesStep::Add), record_id, &a, &b, @@ -281,7 +281,7 @@ where Ok(sum) } else { assert!( - OV::BITS <= SixteenBitStep::max_bit_depth(), + OV::BITS <= SixteenBitStep::BITS, "SixteenBitStep not large enough to accomodate this sum" ); integer_sat_add::<_, SixteenBitStep, B>( diff --git a/ipa-core/src/protocol/ipa_prf/aggregation/step.rs b/ipa-core/src/protocol/ipa_prf/aggregation/step.rs index 39d6927c5..8efb2c392 100644 --- a/ipa-core/src/protocol/ipa_prf/aggregation/step.rs +++ b/ipa-core/src/protocol/ipa_prf/aggregation/step.rs @@ -9,28 +9,19 @@ pub(crate) enum AggregationStep { } #[derive(CompactStep)] -pub enum BucketStep { - /// should be equal to `MAX_BREAKDOWNS` - #[step(count = 512, child = crate::protocol::boolean::step::BoolAndStep)] - Bit(usize), -} - -impl From for BucketStep { - fn from(v: u32) -> Self { - Self::Bit(usize::try_from(v).unwrap()) - } -} +#[step(count = 512, child = crate::protocol::boolean::step::EightBitStep, name = "b")] +pub struct BucketStep(usize); impl From for BucketStep { fn from(v: usize) -> Self { - Self::Bit(v) + Self(v) } } #[derive(CompactStep)] pub(crate) enum AggregateValuesStep { #[step(child = crate::protocol::boolean::step::SixteenBitStep)] - OverflowingAdd, + Add, #[step(child = crate::protocol::ipa_prf::boolean_ops::step::SaturatedAdditionStep)] SaturatingAdd, } diff --git a/ipa-core/src/protocol/ipa_prf/boolean_ops/addition_sequential.rs b/ipa-core/src/protocol/ipa_prf/boolean_ops/addition_sequential.rs index f39f4c5ff..2b47f7a5c 100644 --- a/ipa-core/src/protocol/ipa_prf/boolean_ops/addition_sequential.rs +++ b/ipa-core/src/protocol/ipa_prf/boolean_ops/addition_sequential.rs @@ -8,7 +8,7 @@ use crate::{ helpers::repeat_n, protocol::{ basics::{BooleanProtocols, SecureMul}, - boolean::{or::bool_or, BitStep}, + boolean::{or::bool_or, NBitStep}, context::{Context, UpgradedSemiHonestContext}, Gate, RecordId, }, @@ -38,7 +38,7 @@ pub async fn integer_add( > where C: Context, - S: BitStep, + S: NBitStep, Boolean: FieldSimd, AdditiveShare: BooleanProtocols, Gate: StepNarrow, @@ -61,7 +61,7 @@ pub async fn integer_sat_add<'a, SH, S, const N: usize>( ) -> Result>, Error> where SH: ShardBinding, - S: BitStep, + S: NBitStep, Boolean: FieldSimd, AdditiveShare: BooleanProtocols, N>, Gate: StepNarrow, @@ -100,7 +100,7 @@ async fn addition_circuit( ) -> Result>, Error> where C: Context, - S: BitStep, + S: NBitStep, Boolean: FieldSimd, AdditiveShare: BooleanProtocols, Gate: StepNarrow, diff --git a/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs b/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs index d26b3977d..f744a2d9a 100644 --- a/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs +++ b/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs @@ -13,7 +13,7 @@ use crate::{ ff::{boolean::Boolean, ArrayAccessRef, CustomArray, Field}, protocol::{ basics::{select, BooleanArrayMul, BooleanProtocols, SecureMul, ShareKnownValue}, - boolean::BitStep, + boolean::NBitStep, context::{Context, SemiHonestContext}, Gate, RecordId, }, @@ -36,7 +36,7 @@ pub async fn compare_geq( ) -> Result, Error> where C: Context, - S: BitStep, + S: NBitStep, AdditiveShare: BooleanProtocols, Gate: StepNarrow, { @@ -60,7 +60,7 @@ pub async fn compare_gt( ) -> Result, Error> where C: Context, - S: BitStep, + S: NBitStep, Boolean: FieldSimd, AdditiveShare: BooleanProtocols, Gate: StepNarrow, @@ -85,7 +85,7 @@ pub async fn integer_sub( ) -> Result>, Error> where C: Context, - S: BitStep, + S: NBitStep, AdditiveShare: BooleanProtocols, Gate: StepNarrow, { @@ -108,7 +108,7 @@ pub async fn integer_sat_sub( ) -> Result, Error> where S: SharedValue + CustomArray, - St: BitStep, + St: NBitStep, for<'a> AdditiveShare: BooleanArrayMul>, Gate: StepNarrow, { @@ -154,7 +154,7 @@ async fn subtraction_circuit( ) -> Result>, Error> where C: Context, - S: BitStep, + S: NBitStep, Boolean: FieldSimd, AdditiveShare: BooleanProtocols, Gate: StepNarrow, diff --git a/ipa-core/src/protocol/ipa_prf/boolean_ops/step.rs b/ipa-core/src/protocol/ipa_prf/boolean_ops/step.rs index b3f38d527..c4d10bdf6 100644 --- a/ipa-core/src/protocol/ipa_prf/boolean_ops/step.rs +++ b/ipa-core/src/protocol/ipa_prf/boolean_ops/step.rs @@ -1,7 +1,7 @@ use ipa_step_derive::CompactStep; /// FIXME: This step is not generic enough to be used in the `saturated_addition` protocol. -/// It constraints the input to be at most 2 bytes and it will panic in runtime if it is greater +/// It constrains the input to be at most 2 bytes and it will panic in runtime if it is greater /// than that. The issue is that compact gate requires concrete type to be put as child. /// If we ever see it being an issue, we should make a few implementations of this similar to what /// we've done for bit steps diff --git a/ipa-core/src/protocol/ipa_prf/prf_sharding/mod.rs b/ipa-core/src/protocol/ipa_prf/prf_sharding/mod.rs index d14792c82..8753539dc 100644 --- a/ipa-core/src/protocol/ipa_prf/prf_sharding/mod.rs +++ b/ipa-core/src/protocol/ipa_prf/prf_sharding/mod.rs @@ -24,7 +24,7 @@ use crate::{ boolean::{ or::or, step::{EightBitStep, ThirtyTwoBitStep}, - BitStep, + NBitStep, }, context::{ Context, SemiHonestContext, UpgradableContext, UpgradedSemiHonestContext, Validator, @@ -38,8 +38,8 @@ use crate::{ }, prf_sharding::step::{ AttributionPerRowStep as PerRowStep, AttributionStep as Step, - AttributionWindowStep as WindowStep, AttributionZeroTriggerStep as ZeroStep, - UserNthRowStep, + AttributionWindowStep as WindowStep, + AttributionZeroOutTriggerStep as ZeroOutTriggerStep, UserNthRowStep, }, AGG_CHUNK, }, @@ -197,7 +197,7 @@ where .await?; assert!( - TV::BITS <= EightBitStep::max_bit_depth(), + TV::BITS <= EightBitStep::BITS, "EightBitStep not large enough to accomodate this sum" ); let (updated_sum, overflow_bit) = integer_add::<_, EightBitStep, 1>( @@ -209,7 +209,7 @@ where .await?; assert!( - TV::BITS <= EightBitStep::max_bit_depth(), + TV::BITS <= EightBitStep::BITS, "EightBitStep not large enough to accomodate this subtraction" ); let (overflow_bit_and_prev_row_not_saturated, difference_to_cap) = try_join( @@ -616,11 +616,11 @@ where let (did_trigger_get_attributed, is_trigger_within_window) = try_join( is_trigger_bit.multiply( ever_encountered_a_source_event, - ctx.narrow(&ZeroStep::DidTriggerGetAttributed), + ctx.narrow(&ZeroOutTriggerStep::DidTriggerGetAttributed), record_id, ), is_trigger_event_within_attribution_window( - ctx.narrow(&ZeroStep::CheckAttributionWindow), + ctx.narrow(&ZeroOutTriggerStep::CheckAttributionWindow), record_id, attribution_window_seconds, trigger_event_timestamp, @@ -631,7 +631,7 @@ where // save 1 multiplication if there is no attribution window let zero_out_flag = if attribution_window_seconds.is_some() { - let c = ctx.narrow(&ZeroStep::AttributedEventCheckFlag); + let c = ctx.narrow(&ZeroOutTriggerStep::AttributedEventCheckFlag); did_trigger_get_attributed .multiply(&is_trigger_within_window, c, record_id) .await? @@ -667,7 +667,7 @@ where { if let Some(attribution_window_seconds) = attribution_window_seconds { assert!( - TS::BITS <= ThirtyTwoBitStep::max_bit_depth(), + TS::BITS <= ThirtyTwoBitStep::BITS, "ThirtyTwoBitStep is not large enough to accomodate this subtraction" ); let time_delta_bits = integer_sub::<_, ThirtyTwoBitStep>( diff --git a/ipa-core/src/protocol/ipa_prf/prf_sharding/step.rs b/ipa-core/src/protocol/ipa_prf/prf_sharding/step.rs index e8d983429..fc9e1858f 100644 --- a/ipa-core/src/protocol/ipa_prf/prf_sharding/step.rs +++ b/ipa-core/src/protocol/ipa_prf/prf_sharding/step.rs @@ -17,7 +17,6 @@ pub(crate) enum AttributionStep { #[step(child = UserNthRowStep)] BinaryValidator, PrimeFieldValidator, - ModulusConvertBreakdownKeyBitsAndTriggerValues, #[step(child = crate::protocol::ipa_prf::aggregation::step::AggregationStep)] Aggregate, } @@ -26,7 +25,7 @@ pub(crate) enum AttributionStep { pub(crate) enum AttributionPerRowStep { EverEncounteredSourceEvent, AttributedBreakdownKey, - #[step(child = AttributionZeroTriggerStep)] + #[step(child = AttributionZeroOutTriggerStep)] AttributedTriggerValue, SourceEventTimestamp, #[step(child = crate::protocol::boolean::step::EightBitStep)] @@ -39,7 +38,7 @@ pub(crate) enum AttributionPerRowStep { } #[derive(CompactStep)] -pub(crate) enum AttributionZeroTriggerStep { +pub(crate) enum AttributionZeroOutTriggerStep { DidTriggerGetAttributed, #[step(child = AttributionWindowStep)] CheckAttributionWindow, diff --git a/ipa-core/src/protocol/ipa_prf/quicksort.rs b/ipa-core/src/protocol/ipa_prf/quicksort.rs index 3564903d9..b04906fe2 100644 --- a/ipa-core/src/protocol/ipa_prf/quicksort.rs +++ b/ipa-core/src/protocol/ipa_prf/quicksort.rs @@ -9,7 +9,7 @@ use crate::{ helpers::stream::{process_stream_by_chunks, ChunkBuffer, TryFlattenItersExt}, protocol::{ basics::Reveal, - boolean::{step::ThirtyTwoBitStep, BitStep}, + boolean::{step::ThirtyTwoBitStep, NBitStep}, context::{Context, SemiHonestContext}, ipa_prf::{ boolean_ops::comparison_and_subtraction_sequential::compare_gt, @@ -168,7 +168,7 @@ where .map(Ok); assert!( - K::BITS <= ThirtyTwoBitStep::max_bit_depth(), + K::BITS <= ThirtyTwoBitStep::BITS, "ThirtyTwoBitStep is not large enough to accommodate this sort" ); let comp: BitVec = seq_join( diff --git a/ipa-core/src/protocol/step.rs b/ipa-core/src/protocol/step.rs index 8e799e7a0..b6815127e 100644 --- a/ipa-core/src/protocol/step.rs +++ b/ipa-core/src/protocol/step.rs @@ -32,8 +32,6 @@ pub enum DeadCodeStep { CheckZero, #[step(child = crate::protocol::basics::mul::step::MaliciousMultiplyStep)] MaliciousMultiply, - #[step(child = crate::protocol::basics::mul::step::UpgradeStep)] - DummyUpgrade, #[step(child = crate::protocol::context::step::UpgradeStep)] UpgradeShare, #[step(child = crate::protocol::context::step::MaliciousProtocolStep)] @@ -46,6 +44,4 @@ pub enum DeadCodeStep { FeatureLabelDotProduct, #[step(child = crate::protocol::context::step::ZeroKnowledgeProofValidateStep)] ZeroKnowledgeProofValidate, - // #[cfg_attr(any(test, feature = "test-fixture"), step(child = crate::test_fixture::step::TestExecutionStep))] - // TestExecution, } diff --git a/ipa-core/src/test_fixture/world.rs b/ipa-core/src/test_fixture/world.rs index b8b368431..9c8e97ac2 100644 --- a/ipa-core/src/test_fixture/world.rs +++ b/ipa-core/src/test_fixture/world.rs @@ -265,6 +265,7 @@ impl TestWorld { self.gate_vendor.current() } + #[must_use] fn next_gate(&self) -> Gate { self.gate_vendor.next() } diff --git a/ipa-step/src/gate.rs b/ipa-step/src/gate.rs index 3688cb5f6..da8571f8d 100644 --- a/ipa-step/src/gate.rs +++ b/ipa-step/src/gate.rs @@ -29,6 +29,7 @@ fn build_narrows( let short_name = t.rsplit_once("::").map_or_else(|| t.as_ref(), |(_a, b)| b); let msg = format!("unexpected narrow for {gate_name}({{s}}) => {short_name}({{ss}})"); syntax.extend(quote! { + #[allow(clippy::too_many_lines)] impl ::ipa_step::StepNarrow<#ty> for #ident { fn narrow(&self, step: &#ty) -> Self { match self.0 { diff --git a/scripts/coverage-ci b/scripts/coverage-ci index 113d17c98..b34bc8920 100755 --- a/scripts/coverage-ci +++ b/scripts/coverage-ci @@ -11,13 +11,12 @@ cargo build --all-targets # Need to be kept in sync manually with tests we run inside check.yml. cargo test -# descriptive-gate is enabled by default +# descriptive-gate does not require a feature flag. for gate in "compact-gate" ""; do cargo test --no-default-features --features "cli web-app real-world-infra test-fixture $gate" done -cargo test --bench oneshot_ipa --no-default-features --features "enable-benches" -- -n 62 -c 16 - -cargo test --bench criterion_arithmetic --no-default-features --features "enable-benches" +cargo test --bench oneshot_ipa --no-default-features --features "enable-benches compact-gate" -- -n 62 -c 16 +cargo test --bench criterion_arithmetic --no-default-features --features "enable-benches compact-gate" cargo llvm-cov report "$@"