Conversation
- Introduce `Expr` macro enum and `OpMacro` trait for macro expansion - Add `OP_CLONE`, `OP_VERIFYSIG`, and `OP_ERR` opcodes - Implement macro expansion for `OP_CLONE` and `OP_VERIFYSIG` in scanner - Remove `Op::True` and `Op::False` variants; use `Op::PushByte` - Update VM execution and tests for macro support and new opcodes
Replace OP_OUT_VERSION with OP_OUT_COUNT, which pushes the number of outputs in the transaction. Remove all code and tests related to OP_SELF_VERSION and OP_OUT_VERSION. Rename OP_PRAGMA_START/END to OP_PRAGMA and OP_ENDPRAGMA for clarity.
determine their commitment scheme
✨ Copilot Agent ReviewSummary
Details
What’s Covered
Recommendations
Confidence
|
The OpMacro trait has been updated to include a `dyn_clone` method. This allows for dynamic cloning of OpMacro implementations, which is necessary for certain macro expansions. The `Expr` enum has also been updated to use `Box<dyn OpMacro>` for its `Iter` variant. This change improves the flexibility and extensibility of the macro system by allowing for more complex macro expansions.
The filter_map call was redundant as `Scanner::new(body).flatten()` already handles the `Op::try_from` conversion internally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR brings the
30-macrosbranch intomainand implements VM-level macros along with a substantial refactor of opcodes, the scanner, and execution logic. These changes implement the design described in #30 and add a number of ergonomics and correctness improvements to the VM.Key changes
helm-core/src/vm/macros.rsand a macro-awareScannerthat yieldsExprvalues. Macros supported include:OP_CLONE— repeat an opcode N timesOP_PRAGMA/OP_ENDPRAGMA— embed a pre-encoded sequence to be re-scannedOP_VERIFYSIG— convenience macro to expand the common signature verification sequencehelm-core/src/vm/op.rs):OutAmt,OutData,OutCommare now zero-argument opcodes that pop the output index at runtime (previously they carried an immediate operand).OutCountopcode (pushes number of outputs in the tx).OP_ERR,OP_VERIFYSIG,OP_PRAGMA,OP_ENDPRAGMA,OP_CLONEand associatedOpvariants.False/TrueOp variants in favor ofOP_PUSH_BYTEuses.helm-core/src/vm/mod.rs):Op(macros are flattened before exec).pop_stackhelper to centralize stack pop + error handling.pop_stackand to pop indices at runtime forOut*ops.OP_ERRhandling as a runtime verification error.check_sig_script()now returns theOP_VERIFYSIGmacro sequence.helm-core/src/vm/scanner.rs) rewritten to support macros and safer byte parsing (no index arithmetic). Several new tests for macro scanning were added.Out*ops,OP_VERIFYSIGexpansion andOP_ERRexpectations).Migration notes
Call sites that previously emitted
OP_OUT_AMT,OP_OUT_DATA, orOP_OUT_COMMwith an immediate index must nowOP_PUSH_BYTE <index>before the opcode. Example:[OP_OUT_AMT, 0][OP_PUSH_BYTE, 0, OP_OUT_AMT]Code/tests that relied on
OP_VERIFYas a single opcode should be updated to expect the new behaviour;OP_VERIFYis implemented as a macro expansion usingOP_ERRfor clearer failure semantics.Files with major changes
helm-core/src/vm/macros.rs(new)helm-core/src/vm/mod.rshelm-core/src/vm/op.rshelm-core/src/vm/scanner.rshelm-core/src/transaction.rs(small import/test adjustments)Why this helps
Request for reviewers
Out*opcodes are compatible with all callers.Closes: #30