Improve consistency with neo's codebase #2
Conversation
This file contains 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
|
@hal0x2328 thoughts? |
|
Looks good to me |
shargon
reviewed
Feb 23, 2020
What do you mean? That checking against fault is the same as checking for HALT? |
|
BREAK is other possible state, so we only should accept HALT |
|
I see, so if both checks are not equivalent, isn't it better to use the check that is used in the neo codebase when verifying if the changes made by the script should be commited? That is, isn't the point of this check to make sure that only script execution results which end up being committed get their events relayed? |
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.
I've been trying to make sure that the NeoPubSub plugin only broadcasts events that are not the result of a FAULTy script execution, and while I think that the current plugin accomplishes this, after checking neo's code I changed the extension's code in order to make sure that only script executions that end up being committed are processed.
Essentially I've replicated the check that is done on neo.
Again, with a high probability this PR doesn't change anything, so feel free to close it. I'm just creating it because after spending some time researching it and implementing this solution I thought it may interesting to other ppl.