Skip to content

Update circuit implementation#167

Merged
ConstanceBeguier merged 13 commits intozsa1from
clean_zsa_circuit_part
Jul 23, 2025
Merged

Update circuit implementation#167
ConstanceBeguier merged 13 commits intozsa1from
clean_zsa_circuit_part

Conversation

@ConstanceBeguier
Copy link
Collaborator

@ConstanceBeguier ConstanceBeguier commented Jun 24, 2025

OrchardFlavor modifications:

  • Remove circuit_flavor field from Unproven struct
  • Remove const FLAVOR from OrchardFlavor trait

OrchardCircuit modifications:

  • Create ZsaAdditionalWitnesses struct.
  • When creating witnesses from OrchardVanilla circuits, panic if the asset is not the native asset or if split_flag is not false.

Other modifications:

  • Reduce diff with upstream
  • Remove some types to improve readability

@ConstanceBeguier ConstanceBeguier force-pushed the clean_zsa_circuit_part branch from fa007b1 to 3dd7af6 Compare June 24, 2025 07:26
@QED-it QED-it deleted a comment from what-the-diff bot Jun 24, 2025
@ConstanceBeguier ConstanceBeguier force-pushed the clean_zsa_circuit_part branch from bad73e1 to 1637e75 Compare July 8, 2025 13:39
@ConstanceBeguier
Copy link
Collaborator Author

Force pushed after rebasing on zsa1 branch

Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

I think Vanilla users should not have an extra "zsa" step. Mentioned some alternatives to the suggested approach.

src/circuit.rs Outdated

// The fields inside `zsa_witnesses` are only populated for OrchardZSA circuits.
// They are `Unknown` in OrchardVanilla circuits.
pub(crate) zsa_witnesses: ZsaWitnesses,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don’t you prefer to make it Option<> instead of assigning unknown on vanilla bundle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer to have only Value (and no Option) in the circuit part.
I replaced ZsaWitnesses struct with ((psi_nf, asset), split_flag) in order to be able to use the unzip function on Value.

///
/// # Panics
/// Panics if the asset is not a native asset or if `split_flag` is true.
fn build_zsa_witnesses(_: pallas::Base, asset: AssetBase, split_flag: bool) -> ZsaWitnesses {
Copy link
Collaborator

Choose a reason for hiding this comment

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

build_zsa_witnesses( for Vanilla does not make alot of sence. Vanilla users should not remember calling this meaningless function.

Alternativly, we get

psi_nf: Value::unknown(),
asset: Value::unknown(),
split_flag: Value::unknown(),

on init and call nothing for vanilla.

Or we have a build_withness( that is mandatory for both Vanilla and ZSA to handle all withnesses.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can move some of this logic to the ZsaWitnesses { constructor that is used only by ZSA users.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The function build_witness already exists. It is called from_action_context_unchecked. Inside this function we call build_zsa_witnesses.

@ConstanceBeguier ConstanceBeguier force-pushed the clean_zsa_circuit_part branch from 522bc37 to 6aabe66 Compare July 15, 2025 08:53
@ConstanceBeguier
Copy link
Collaborator Author

Force pushed after rebasing on zsa1 branch

Remove circuit_flavor field from Unproven struct
Remove const FLAVOR from OrchardFlavor trait

Create ZsaWitnesses struct.
For OrchardVanilla circuits, all fields in ZsaWitnesses are Unknown.
When creating witnesses from OrchardVanilla circuits, panic if the
asset is not the native asset or if split_flag is not false.
In this PR, we reduce diff with upstream and we remove some types to
improve readability.
@ConstanceBeguier ConstanceBeguier force-pushed the clean_zsa_circuit_part branch from ac9e527 to ef7cca0 Compare July 15, 2025 13:01
@ConstanceBeguier
Copy link
Collaborator Author

Force pushed after rebasing on zsa1 branch

Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

very nice, added some more improvments

Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

much better now

@ConstanceBeguier ConstanceBeguier merged commit f7bcf01 into zsa1 Jul 23, 2025
17 checks passed
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.

2 participants