Skip to content

P-token tweaks #598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 23, 2025
Merged

P-token tweaks #598

merged 12 commits into from
May 23, 2025

Conversation

jberthold
Copy link
Member

  • implemented Downcast projection (overwrites the variantIdx of given enum data, without checking)
  • implemented unOpPtrMetadata for references to arrays and slices (returns length as usize)
  • decoding zero-sized structs (returns struct with no fields, type is already set. Not checking for absence of fields atm)
  • thunking for #decode: unimplemented decoding becomes a thunk with bytes and type information

This allows the p-token transfer proof to progress further without spurious branching.

Also removes the vacuous branches in symbolic-args-fail, making more progress because PtrMetadata is now available

@jberthold jberthold requested review from dkcumming and ehildenb May 22, 2025 02:23
Comment on lines +1355 to +1356
rule <k> #arrayLength(TV, PROJS) => #readProjection(TV, PROJS) ~> #arrayLength() ... </k>
requires notBool PROJS ==K .ProjectionElems
Copy link
Member Author

Choose a reason for hiding this comment

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

A word of warning: I had this as an [owise] rule first, and got loads of branching on the thunk default rule for Evaluations. We cannot use owise on rules over Evaluation.

@jberthold jberthold marked this pull request as ready for review May 22, 2025 02:33
Comment on lines 913 to 921
// unimplemented cases stored as thunks
syntax Evaluation ::= Undecoded ( Bytes, TypeInfo )

rule #decodeConstant(constantKindAllocated(allocation(BYTES, _, _, _)), INFO) => thunk(Undecoded(BYTES, INFO)) [owise]

// a rule for zero-sized constants for struct types without fields
rule #decodeConstant(constantKindZeroSized, typeInfoStructType(_, _)) => Aggregate(variantIdx(0), .List)

```
Copy link
Member

Choose a reason for hiding this comment

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

This decodeConstant operates a bit differently than teh other rvalue we are evaluating (the operandConst). All teh other ones go to an Evaluation on the <k>-cell, not as a function, which means we can have a single owise rule for thunking them just because they're in sort Evaluation. Instead here, we are going straight to calling the function #decodeConstant which returns a Value instead of a TypedValue, so thunking has to be done specially for it.

Can we instead have the rule for operandConst be like (with #decodeConstant no longer a function, and have it return a full TypedValue):

  rule <k> operandConstant(constOperand(_, _, mirConst(KIND, TY, _))) 
        => #decodeConstant(KIND, {TYPEMAP[TY]}:>TypeInfo), TY) 
        ... 
      </k> 
      <types> TYPEMAP </types> 
    requires TY in_keys(TYPEMAP) 
     andBool isTypeInfo(TYPEMAP[TY]) 
    [preserves-definedness] // valid Map lookup checked 

Then we don't need a special case for thunking it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe indeed decodeConstant should also return TypedValue for now.
We should also do the refactoring to make them all return Value at some point soon, but I will adapt decodeConstant for now.

Copy link
Member

@ehildenb ehildenb left a comment

Choose a reason for hiding this comment

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

Looks good overall! I'm going to open a separate PR to shorten the output, and have one question/thought about the #decodeConstant (up to you how to handle it).

jberthold and others added 3 commits May 23, 2025 10:33
Conflicts:
	kmir/src/tests/integration/data/prove-smir/show/pinocchio_token_program.smir.processor::transfer::process_transfer.expected
@jberthold jberthold merged commit 5398bb5 into master May 23, 2025
6 checks passed
@jberthold jberthold deleted the jb/p-token-tweaks branch May 23, 2025 02:49
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