argumentcomputer / argumentcomputer/sphinx
Modularize WP1 Air traits
- Dominant language
- Rust
- Stars
- 80
- Forks
- 8
- PR merge metrics
- No merged PRs in 30d
Description
There are several modularity issues with the WP1 Air trait organization, mostly defined in [core::air::builder](https://github.com/wormhole-foundation/wp1/blob/dev/core/src/air/builder.rs).
1. the convenience trait `SP1AirBuilder` is used in bound position very often, whereas a more sparse trait would suffice. This isn't a problem for the SP1 code base since this convenience trait is auto-implemented, but muddles that the implementations often have simpler requirement. For instance:
https://github.com/wormhole-foundation/wp1/blob/ef5133d62b2575a8e75fd35b0d6a1bf83d7cd823/core/src/alu/add_sub/mod.rs#L140-L142
probably only requires an `AluAirBuilder`.
2. We would like to reuse the existing definitions of `generate_trace` for gadgets. Those are defined in the `MachineAir` trait, which requires a `generate_trace` that's a bit inflexible: in the form of a single instantiation of a `Self::Record: MachineRecord` type:
a. the `Self::Record` type is bound once and for all at the implementation of the `MachineAir` trait. We anticipate that a generic `generate_trace` which takes on a parameter `MR: AdvancedMachineRecord` with `AdvancedMachineRecord` a suitable bound would be more flexible,
b. the `generate_trace` implementations are specialized to work on the `ExecutionRecord`, a concrete, SP1-specific-type and would probably be more reusable if they worked against a trait that abstracts the "common denominator" fields required of this (the above `AdvancedMachineRecord`) + some specific traits that would encode the capability of storing gadget-specific events (e.g. events for point decompression specific to a gadget accomplishing that on a specific curve).
Contributor guide
Research direction
Start with core/src/air/builder.rs and the referenced core/src/alu/add_sub/mod.rs bounds to map the current trait relationships. Then trace MachineAir::generate_trace, ExecutionRecord, and the record traits mentioned in the issue. Done means the air traits and trace generation support the proposed reuse without unnecessary bounds, with affected tests and implementations still passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100