-
Notifications
You must be signed in to change notification settings - Fork 99
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: support ballot #454
base: feat/host-handlers
Are you sure you want to change the base?
feat: support ballot #454
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems the requirement is not clear, some codes need to be changed.
- ZK draw works as a hit-or-miss draw, where a successful draw applies ZK, and an unsuccessful draw does not, so no need to fall back to that always type.
- because of 1, you need to respond to client with a draw fail flag to let them know no ZK for this block.
- zk draw will be used together with ZK_ANY type because no on-chain hint for zk type anymore.
- The input/output need change according to 3, for input, you need to support ZK_ANY, for output, you need to tell client what type this proof is using.
impl Ballot { | ||
/// Create a new Ballot | ||
pub fn new( | ||
always_proof_type: ProofType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's that always_proof_type? ZK_ANY??
|
||
// Validate each probability | ||
for (&proof_type, &prob) in self.probabilities.iter() { | ||
if !(0.0..=1.0).contains(&prob) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
float range looks weird, suggest use traditional <= check.
let mut cumulative_prob = 0.0; | ||
for (proof_type, &prob) in self.probabilities.iter() { | ||
cumulative_prob += prob; | ||
if remainder < (cumulative_prob * PRECISION as f64).round() as u64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming sp1 is 5% and r0 is also 5%, if you use 10000 as the precision value (PRECISION), the first 500 blocks will be assigned to sp1, the next 500 to risc0, and the remaining 9000 blocks will remain idle. Although the ratio is still 10%, this large-scale burst behavior doesn't seem ideal (for now), as managing 500 remote ZK tasks can be error-prone.
Especially in product env, I recommend adopting a more time-distributed draw instead of this burst allocation. However, before we find a better solution, using 100 as the precision value seems also acceptable, as handling a burst of 5/5 tasks and idol for 90 is much easier
No description provided.