feat: remove FALCON_SIG_TO_STACK event
#1703
Conversation
processor/src/host/mod.rs
Outdated
| // key is in the advice provider and uses it to in signature generation | ||
| #[cfg(any(test, feature = "testing"))] | ||
| if event_id == crate::utils::EVENT_FALCON_SIG_TO_STACK { | ||
| let advice_provider = self.advice_provider_mut(); | ||
| push_signature(advice_provider, process, SignatureKind::RpoFalcon512) | ||
| } else { | ||
| #[cfg(feature = "std")] | ||
| std::println!( | ||
| "Event with id {} emitted at step {} in context {}", | ||
| event_id, | ||
| process.clk(), | ||
| process.ctx() | ||
| ); | ||
| Ok(()) | ||
| return test::push_falcon_signature(advice_provider, process); | ||
| } |
There was a problem hiding this comment.
I put this here since our Test struct uses DefaultHost instead of some TestHost. But I don't think anyone uses DefaultHost anyways, so its main use is probably just tests
There was a problem hiding this comment.
I would maybe add a very brief comment here as well (basically, saying what you said in the comment above).
plafer
force-pushed
the
plafer-cleanup-falcon-event
branch
from
f49818b to
eeca630
Compare
There was a problem hiding this comment.
Thank you! Looks good! I added some comments inline - but nothing too significant.
We'll probably also need a parallel PR in miden-base to introduce the signature verification event there.
The issue with #1647 is that we still don't package the event handlers with the library. Hence, users still cannot define event handlers with their libraries, since they won't be able to ship them e.g. to some delegated prover.
I think we still may want to do this - but the event handlers could probably be much simpler. So, I wouldn't close #1584 just yet.
processor/src/host/mod.rs
Outdated
| // key is in the advice provider and uses it to in signature generation | ||
| #[cfg(any(test, feature = "testing"))] | ||
| if event_id == crate::utils::EVENT_FALCON_SIG_TO_STACK { | ||
| let advice_provider = self.advice_provider_mut(); | ||
| push_signature(advice_provider, process, SignatureKind::RpoFalcon512) | ||
| } else { | ||
| #[cfg(feature = "std")] | ||
| std::println!( | ||
| "Event with id {} emitted at step {} in context {}", | ||
| event_id, | ||
| process.clk(), | ||
| process.ctx() | ||
| ); | ||
| Ok(()) | ||
| return test::push_falcon_signature(advice_provider, process); | ||
| } |
There was a problem hiding this comment.
I would maybe add a very brief comment here as well (basically, saying what you said in the comment above).
|
Also added |
|
|
Supersedes #1647
The issue with #1647 is that we still don't package the event handlers with the library. Hence, users still cannot define event handlers with their libraries, since they won't be able to ship them e.g. to some delegated prover.
This PR then applies this suggestion in isolation, effectively cleaning up the tech debt associated with the
FALCON_SIG_TO_STACKevent.