Skip to content

Use ExpectedTranscriptError in Orchard ZSA workflow block tests#133

Open
dmidem wants to merge 6 commits into
zsa1from
expected-transcript-error
Open

Use ExpectedTranscriptError in Orchard ZSA workflow block tests#133
dmidem wants to merge 6 commits into
zsa1from
expected-transcript-error

Conversation

@dmidem

@dmidem dmidem commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

This PR replaces the is_valid: bool field in OrchardWorkflowBlock with an explicit Result<(), ExpectedTranscriptError> as discussed in issue #116.

The original test gap was that is_valid: false only checked that block 5 failed, but did not check why it failed. Ideally, this test should assert that block 5 is rejected specifically because it tries to issue more assets after issuance was finalized.

I tried to make the workflow vector check that exact error, but the current transcript test API only supports ExpectedTranscriptError::Any and ExpectedTranscriptError::Exact(...). For this rejection path, there is no specific error currently exposed through the transcript checker that can be matched here, so block 5 still uses Err(ExpectedTranscriptError::Any).

So this PR does not fully add the semantic “post-finalization issuance” check yet, but it does remove the boolean validity flag and changes the vector format to use the typed transcript expected-result API. This makes the test data clearer and leaves the code ready for a stricter expected error once the transcript checker can expose one.

@dmidem dmidem requested a review from PaulLaux June 1, 2026 08:31
@dmidem dmidem force-pushed the sync-zcash-v4.2.0-merge branch from bb1b718 to a90946a Compare June 1, 2026 21:38
@dmidem dmidem changed the base branch from sync-zcash-v4.2.0-merge to zsa1 June 2, 2026 07:31

@PaulLaux PaulLaux left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See comment

fi

sleep 30
done

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These changes does not seem to be related to PR title. should move it to a separate PR / rollback ?

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