Generic WeightedConfig
#18141
Generic WeightedConfig
#18141
Conversation
rex1fernando
requested review from
alinush,
ibalajiarun,
sasha8,
zekun000 and
zjma
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| /// Weighted config for the Arkworks-based PVSS | ||
| #[allow(type_alias_bounds)] | ||
| pub type WeightedConfigArkworks<F: FftField> = WeightedConfig<ShamirThresholdConfig<F>>; |
There was a problem hiding this comment.
Bug: Trait Bound Mismatch Permits Invalid Type Aliases
The WeightedConfigArkworks type alias uses F: FftField as its bound, but ShamirThresholdConfig<F> only implements ThresholdConfig when F: PrimeField. Since WeightedConfig<TC> requires TC: ThresholdConfig, this type alias allows constructing invalid types where the trait bound isn't satisfied. The bound should be F: PrimeField instead of F: FftField.
| } | ||
|
|
||
| let tc = ThresholdConfigBlstrs::new(threshold_weight, W)?; | ||
| let tc = TC::new(threshold_weight, W)?; |
There was a problem hiding this comment.
Last time we discussed the ThresholdConfig trait I thought the consensus was to move or remove the new() method... but I think it works here?
There was a problem hiding this comment.
Yeah, it ended up being useful here.
My original idea was to define a single ThresholdConfig struct that's generic over an evaluation domain; then we'd be able to use it both for blstrs and for arkworks, and we wouldn't need a trait. But the blstrs version has both an EvaluationDomain and a BatchEvaluationDomain, so I wasn't sure how to do this cleanly...
Description
Introduces a new
WeightedConfigtype that is generic over botharkworksandblstrs.Stacks on top of #18102.
How Has This Been Tested?
Same tests as in the original
weighted_config.rsKey Areas to Review
Now that
WeightedConfigapplies to botharkworksandblstrs, we should move it out ofsrc/blstrs. Where should we move it to? @alinush @waammType of Change
Which Components or Systems Does This Change Impact?
aptos-cryptoNote
Make WeightedConfig generic over threshold backends and update all DKG/WVUF/consensus code to use the blstrs alias (WeightedConfigBlstrs) with minor API adjustments.
WeightedConfig<TC: ThresholdConfig>with aliasesWeightedConfigBlstrsandWeightedConfigArkworks.tc.t/n) with trait getters; addDisplay/SecretSharingConfigimpls for generic type.WeightedConfigBlstrs.WeightedConfigtoWeightedConfigBlstrs(traits, transcripts, helpers, benches, tests).Transcript::SecretSharingConfig, bench helpers) accordingly.RandConfig.wconfigand related imports/usages toWeightedConfigBlstrs.WeightedConfigBlstrsforwconfig/fast_wconfigconstruction and storage.Written by Cursor Bugbot for commit ba21684. This will update automatically on new commits. Configure here.