-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix: Use Interpreter without building Tx or Output #60
Conversation
Codecov Report
@@ Coverage Diff @@
## master #60 +/- ##
==========================================
+ Coverage 84.53% 84.76% +0.22%
==========================================
Files 27 28 +1
Lines 2943 3012 +69
==========================================
+ Hits 2488 2553 +65
- Misses 320 325 +5
+ Partials 135 134 -1
Continue to review full report at Codecov.
|
bscript/interpreter/engine.go
Outdated
@@ -22,13 +22,29 @@ func NewEngine() Engine { | |||
|
|||
// Execute will execute all scripts in the script engine and return either nil | |||
// for successful validation or an error if one occurred. | |||
// | |||
// Execute with tx example: | |||
// if err := engine.Execute(interpreter.ExecutionParams{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be better using something like this engine.Execute(WithTx(tx, previousOutput,...))
and engine.Execute(WithScripts(lockingScript, unlockingScript,...))
.
My thought is with the single struct as shown, could be confusing and a user may see all options and think they may all be required ie tx AND script.
I would propose the With func would not be variadic but a single function accepted by the executor, so they also don't do WithTx AND WithScript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aw good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Currently the user is expected to build a full tx and output in order to use the interpreter, even if the script doesn't check sigs.
To address this I've added:
LockingScript
andUnlockingScript
to the execution params