-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: gas hook on ExecutionFinalResult #284
Conversation
@frol Since I can't invite reviewers currently, any comments from reviewers are welcome. |
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.
@shariffdev I don't think it is what was described in the issue, see my comment there: #29 (comment)
Checking right now
|
From what I see, this is only hooking onto the current transaction. We want to have the callback be executed on all transactions since the creation of the What I suggest is to have a very similar API to Rust's Instant so that when it's created via |
Hi @ChaoticTempest can you check this |
@shariffdev Please, make the CI pass. I see there is a dependency issue, use the following workaround: https://github.com/near/near-sdk-rs/pull/1075/files#diff-748c9b7c0e525461124dec4c258406a92d81a9b610f7b47a79efe9a0cb52553c |
This PR should take care of the dependency issue as we are using a newer stable toolchain |
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.
Aside from the error unwrappings, this one is good to go
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.
@shariffdev Thanks for working on it and adding the documentation with an actionable example!
Closes #29
This approach as suggested by @ChaoticTempest does not add state to theworker
and a hook can be offered instead, the theon_transaction
method takes a hook that can collect the gas burnt information.AGasMeter
type is added with updated test code on how this all comes together to meter the gas burnt on doing asynchronous calls.This approach allows to aggregate
total_gas_burnt
from all transactions that theWorker
sees while transactions are being called.