Skip to content

chore(backend): implement more circuit op logic#173

Merged
ax0 merged 8 commits into0xPARC:mainfrom
ax0:mainpod_notcontains
Apr 7, 2025
Merged

chore(backend): implement more circuit op logic#173
ax0 merged 8 commits into0xPARC:mainfrom
ax0:mainpod_notcontains

Conversation

@ax0
Copy link
Collaborator

@ax0 ax0 commented Apr 2, 2025

Towards #156.

@ax0 ax0 marked this pull request as ready for review April 2, 2025 20:47
@ax0 ax0 requested review from arnaucube and ed255 April 4, 2025 19:00
Copy link
Collaborator

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM!
I left a few minor suggestions

// of the provided Merkle proofs (if any). These proofs have already
// been verified, so we need only look up the claim.
let resolved_merkle_claim =
(merkle_claims.len() > 0).then(|| builder.vec_ref(merkle_claims, op.aux[0]));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will merkle_claims.len() ever be 0?
If that's not the case, this could be simplified.

params,
&statements,
&[], // TODO: fill in the merkle proofs for Contains/NotContains ops
&merkle_proofs, // TODO: fill in the merkle proofs for Contains/NotContains ops
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 now remove the TODO?

impl TryFrom<MerkleClaimAndProof> for merkletree::MerkleProof {
type Error = anyhow::Error;
fn try_from(mp: MerkleClaimAndProof) -> Result<Self> {
match mp.enabled {
Copy link
Collaborator

Choose a reason for hiding this comment

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

A suggestion to avoid too much nesting is to follow this pattern:

if bad_condition {
  return Error;
}
// Proceed with the happy flow

@ax0 ax0 merged commit 6528914 into 0xPARC:main Apr 7, 2025
4 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.

3 participants