Skip to content

smplx-build: fix witness type conversion in single-valued either type#51

Open
ikripaka wants to merge 1 commit intodevfrom
fix/include-simf-single-either-gen
Open

smplx-build: fix witness type conversion in single-valued either type#51
ikripaka wants to merge 1 commit intodevfrom
fix/include-simf-single-either-gen

Conversation

@ikripaka
Copy link
Copy Markdown
Collaborator

  • This PR suggests a bug fix and I've added the necessary tests.
  • This PR introduces a new feature and I've discussed the update in an Issue or with the team.
  • This PR is just a minor change like a typo fix.

PR fixes an error in include_simf! macro when handling single-valued Either or Option types in the Simplicity contract.

fn main () {
    let owner_sig: Signature = witness::SIGNATURE;

    match witness::PATH {
        Left(x: (Either<u16, u64>, Either<Option<bool>, Option<u256>>)) => {
            let (either1, either2): (Either<u16, u64>, Either<Option<bool>, Option<u256>>) = x;
        },
        Right(x: Either<u32, Either<u128, u8>>) => {
            let either1: Either<u32, Either<u128, u8>> = x;
        },
    }
}

include_simf! was incorrectly generating references for nested values in Option and Either<T, U>. After expansion, the generated code was failing because it passed &T value to Value::from instead of the owned T type.

impl simplex::program::WitnessTrait for EitherWithSingleWitnessWitness {
            #[doc = r" Build Simplicity witness values for contract execution."]
            #[must_use]
            fn build_witness(&self) -> simplex::simplicityhl::WitnessValues {

            //...
                  match &left_val.1 {
                            simplex::either::Either::Left(left_val) => {
                                Value::left(match &left_val {
                                    None => { Value::none(ResolvedType::boolean()) }
                                    Some(inner_val) => { Value::some(Value::from(inner_val)) }
                                }, ResolvedType::option(ResolvedType::u256()))
                            }
                            simplex::either::Either::Right(right_val) => {
                                Value::right(ResolvedType::option(ResolvedType::boolean()), match &right_val {
                                    None => { Value::none(ResolvedType::u256()) }
                                    Some(inner_val) => { Value::some(Value::from(UIntValue::U256(U256::from_byte_array(inner_val)))) }
                                })
                            }
                        }
            //...
            }
}

This PR updated the macro to generate explicit dereferences *inner_val, correctly passing the underlying values.

impl simplex::program::WitnessTrait for EitherWithSingleWitnessWitness {
            #[doc = r" Build Simplicity witness values for contract execution."]
            #[must_use]
            fn build_witness(&self) -> simplex::simplicityhl::WitnessValues {

            //...
                  match &left_val.1 {
                            simplex::either::Either::Left(left_val) => {
                                Value::left(match &left_val {
                                    None => { Value::none(ResolvedType::boolean()) }
                                    Some(inner_val) => { Value::some(Value::from(*inner_val)) }
                                }, ResolvedType::option(ResolvedType::u256()))
                            }
                            simplex::either::Either::Right(right_val) => {
                                Value::right(ResolvedType::option(ResolvedType::boolean()), match &right_val {
                                    None => { Value::none(ResolvedType::u256()) }
                                    Some(inner_val) => { Value::some(Value::from(UIntValue::U256(U256::from_byte_array(*inner_val)))) }
                                })
                            }
                        }
            //...
            }
}

Why this approach?
We can solve this issue without changing the interface in different ways

  1. invoke .to_owned() on each path
    caveats: double cloning
  2. remove references on each match match &val {} -> match val {}
    caveats: possible redundant object copying
  3. include explicit dereference in needed places
    By far, this is the most efficient way to fix the issue, as it avoids unnecessary copying of types.

@ikripaka ikripaka self-assigned this Apr 17, 2026
@ikripaka ikripaka requested a review from Arvolear as a code owner April 17, 2026 07:05
@ikripaka ikripaka added the bug Something isn't working label Apr 17, 2026
@Arvolear
Copy link
Copy Markdown
Collaborator

Looks good! Let's fix other incompatibility issues here as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants