Skip to content
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

Atleth bonding accounting #76

Merged
merged 20 commits into from
Dec 13, 2023
Merged

Conversation

thogard785
Copy link
Contributor

WIP

note the _assign() and _credit() methods in GasAccounting.

if (_deposits < _claims + _withdrawals) {
// CASE: in deficit, subtract from bonded balance
uint256 amountOwed = _claims + _withdrawals - _deposits;
if (_assign(winningSolver, amountOwed)) revert InsufficientTotalBalance((_claims + _withdrawals) - deposits);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use amountOwed here as its calculated the same way, just above?

Suggested change
if (_assign(winningSolver, amountOwed)) revert InsufficientTotalBalance((_claims + _withdrawals) - deposits);
if (_assign(winningSolver, amountOwed)) revert InsufficientTotalBalance(amountOwed);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so amountOwed uses _deposits, a memory variable, whereas the revert uses deposits, which is a storage variable. The reason the revert uses the storage variable is because _assign() increments it up to where we need it to be to calculate the shortfall rather than the total amount owed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it may be redundant though... that storage write in _assign doesn't need to happen at the end. There's probably a better way to do it.

Copy link
Contributor

@BenSparksCode BenSparksCode left a comment

Choose a reason for hiding this comment

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

Looks good so far, I think it simplifies AtlETH and GasAccounting a lot

@thogard785 thogard785 marked this pull request as ready for review December 13, 2023 07:01
Copy link
Contributor

@BenSparksCode BenSparksCode left a comment

Choose a reason for hiding this comment

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

LGTM!

@BenSparksCode BenSparksCode merged commit c6a593c into atleth-bonding Dec 13, 2023
3 checks passed
@BenSparksCode BenSparksCode deleted the atleth-bonding-accounting branch December 13, 2023 07:53
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.

2 participants