argumentcomputer / argumentcomputer/sphinx

Modularize WP1 Air traits

Open
#15 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.