Skip to content

Make t and allowed_delta configurable#413

Merged
zhouwfang merged 3 commits intomainfrom
zhoufang/iop-116-make-t-and-allowed_delta-configurable
Apr 15, 2026
Merged

Make t and allowed_delta configurable#413
zhouwfang merged 3 commits intomainfrom
zhoufang/iop-116-make-t-and-allowed_delta-configurable

Conversation

@zhouwfang
Copy link
Copy Markdown
Contributor

@zhouwfang zhouwfang requested review from benr-ml and jonas-lj April 6, 2026 22:36
@zhouwfang zhouwfang requested a review from bmwill as a code owner April 6, 2026 22:36
@zhouwfang zhouwfang force-pushed the zhoufang/iop-116-make-t-and-allowed_delta-configurable branch 2 times, most recently from d5e011c to 1cca276 Compare April 6, 2026 22:40
Copy link
Copy Markdown
Contributor

@bmwill bmwill left a comment

Choose a reason for hiding this comment

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

move changes lgtm @Bridgerz want to take a look as well?

let (nodes, threshold) = build_reduced_nodes(&committee, allowed_delta, weight_divisor)?;
let (nodes, threshold) = build_reduced_nodes(
&committee,
threshold_basis_points,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name confused me a bit. You call it basis points here because you're defining the threshold as some ratio with a fixed denominator, MAX_BASIS_POINTS, and then use this to compute the threshold with the actual given weights, right? if so, I'd call this something else, like TOTAL_BASIS_WEIGHT, or just document the relationship with THRESHOLD_BASIS_POINTS.

Copy link
Copy Markdown
Contributor Author

@zhouwfang zhouwfang Apr 14, 2026

Choose a reason for hiding this comment

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

I renamed threshold_basis_points to be threshold_in_basis_points to make it clear this is "the threshold, expressed in basis points" rather than "basis points of the threshold." I kept MAX_BASIS_POINTS rather than TOTAL_BASIS_WEIGHT, which sounds like a sum of something that could vary rather than a fixed constant.

@zhouwfang zhouwfang force-pushed the zhoufang/iop-116-make-t-and-allowed_delta-configurable branch from 4f79ce9 to dfe42fc Compare April 14, 2026 22:05
@zhouwfang zhouwfang requested a review from jonas-lj April 14, 2026 22:06
@zhouwfang zhouwfang force-pushed the zhoufang/iop-116-make-t-and-allowed_delta-configurable branch from 4bd3b8a to dfe42fc Compare April 15, 2026 00:19
@zhouwfang zhouwfang merged commit da4f17c into main Apr 15, 2026
11 of 12 checks passed
@zhouwfang zhouwfang deleted the zhoufang/iop-116-make-t-and-allowed_delta-configurable branch April 15, 2026 17:46
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.

3 participants