Skip to content

Conversation

@jayantk
Copy link
Contributor

@jayantk jayantk commented May 8, 2025

Summary

As the title says. Track gas usage and emit it in the event.

Rationale

I previously omitted gas tracking in this recovery flow (which is identical to the old callback flow). However, I realized tracking gas will be useful here -- if a user's callback fails and they recover it, they probably want to know how much gas it used so they can set the gas limit better next time.

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

@vercel
Copy link

vercel bot commented May 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api-reference ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm
component-library ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm
entropy-debugger ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm
entropy-explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm
insights ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm
proposals ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm
staking ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2025 7:36pm

EntropyStructs.Request memory reqV1 = EntropyStructConverter
.toV1Request(req);
clearRequest(provider, sequenceNumber);
// WARNING: DO NOT USE req BELOW HERE AS ITS CONTENTS HAS BEEN CLEARED
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

referencing req.requester in the prior version of this code wasn't a good idea. (that field doesn't get cleared, but not good to make assumptions about that)


// callback gas usage is approximate and only triggered when the provider has set a gas limit.
// Note: this condition is somewhat janky, but we hit the stack limit so can't put in any more local variables :(
// callback gas usage is approximate
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that other tests of the Revealed event don't check the gas field because it's too hard to predict up front. Thus, we're using vm.expectEmit to only check the topics not the data.

@jayantk jayantk merged commit 89b1f2a into main May 9, 2025
11 checks passed
@jayantk jayantk deleted the entropy_error16 branch May 9, 2025 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants