Skip to content

Conversation

rakita
Copy link
Member

@rakita rakita commented Sep 30, 2025

Helper function as we have three places where value needs to be deducted.

Exctracted from #2991

@rakita rakita requested review from klkvr and mattsse September 30, 2025 20:51
Copy link

codspeed-hq bot commented Sep 30, 2025

CodSpeed Performance Report

Merging #3030 will not alter performance

Comparing rakita/helper-effective_balance_spending_without_value (e3ae0dc) with main (a32f8f4)

Summary

✅ 173 untouched

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

makes sense to me, some doc nits

Copy link
Collaborator

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

I'm personally fine with keeping this duplicated

@rakita
Copy link
Member Author

rakita commented Oct 1, 2025

I'm personally fine with keeping this duplicated what did you mean by duplicated?

@rakita rakita requested review from klkvr and mattsse October 1, 2025 22:11
@klkvr
Copy link
Collaborator

klkvr commented Oct 2, 2025

I'm personally fine with keeping this duplicated what did you mean by duplicated?

I meant that this is just a single line of code deduplicating which might not be worth the indirection and a new trait method

@rakita
Copy link
Member Author

rakita commented Oct 2, 2025

I see, as it is a trait method with a default implementation, it will not require reimplementation it. For this case it made sense to add it.

It does replace fetching two values from the same place tx.effective_*() - tx.value with one fetch tx.gas_balance_*()

@rakita rakita merged commit ef98432 into main Oct 2, 2025
31 checks passed
@github-actions github-actions bot mentioned this pull request Oct 5, 2025
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