Parse logged types #582
Merged
Parse logged types #582
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
Co-authored-by: Halil Beglerović <hal3e.git@gmail.com>
Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
Co-authored-by: Halil Beglerović <hal3e.git@gmail.com>
Co-authored-by: Halil Beglerović <hal3e.git@gmail.com>
Co-authored-by: Halil Beglerović <hal3e.git@gmail.com>
Co-authored-by: Halil Beglerović <hal3e.git@gmail.com>
Co-authored-by: Halil Beglerović <hal3e.git@gmail.com>
Co-authored-by: Ahmed Sagdati <37515857+segfault-magnet@users.noreply.github.com>
segfault-magnet
previously approved these changes
Oct 3, 2022
digorithm
requested changes
Oct 3, 2022
There was a problem hiding this comment.
This looks amazing. I have one issue with the semantics, though. Whenever I call something with print in it, I expect it to automatically print something to stdout/stderr.
print_logs() doesn't print anything; it returns a Vec<String>. The name/behavior is misleading.
I propose we rename print_logs() to fetch_logs()/get_logs()/resolve_logs() and keep the same behavior (return a Vec<String> and let the user decide what to do with it.)
|
|
||
| Ok(()) | ||
| } | ||
|
|
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.
Closes #562
This PR adds
_logs_with_type::<T>()to the existing contract instance methods. It requires receipts and returns aVec<T>containing all the logged values of typeT.It also adds
_print_logs()for outputting all the logged values to stdout.The feature flag
--generate-logged-typesis added to the build test script so that we can test, but needs to be removed as soon as forc starts generating logged types without it.As mentioned in the issue, ABI support for logging generic types is limited. However, it is likely that once added, it will work on the SDK side with little to no changes.
Another future upgrade we can expect is to further filter the logs based on the name of the logged variables once they are included in the ABI.
I will add the follow-up issues once we agree on the changes.