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.

Sign up for GitHub

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

ledger: disable minimum balance check when not validating or generating #2149

Merged
merged 4 commits into from
May 18, 2021
Merged

ledger: disable minimum balance check when not validating or generating #2149

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf19a5b
Disable minimum balance check when not validating or generating.
tolikzinovyev May 13, 2021
12804c1
Move the loop to a different function.
tolikzinovyev May 14, 2021
238599e
Retain the original error message.
tolikzinovyev May 17, 2021
e05b727
Run gofmt.
tolikzinovyev May 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions ledger/eval.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,17 +859,22 @@ func (eval *BlockEvaluator) transaction(txn transactions.SignedTxn, evalParams *
continue
}

dataNew := data.WithUpdatedRewards(eval.proto, rewardlvl)
effectiveMinBalance := dataNew.MinBalance(&eval.proto)
if dataNew.MicroAlgos.Raw < effectiveMinBalance.Raw {
return fmt.Errorf("transaction %v: account %v balance %d below min %d (%d assets)",
txid, addr, dataNew.MicroAlgos.Raw, effectiveMinBalance.Raw, len(dataNew.Assets))
}
// Only do those checks if needed. It is useful to skip them if the user cannot provide
// account data that contains enough information to compute the correct minimum balance
// (the case with indexer).
if eval.validate || eval.generate {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid the indentation to match the rest of the code by changing this to

if !eval.validate && !eval.generate {
    continue
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's common to have an if statement for eval.validate or eval.generate. I also oppose continue and break. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should take one step back here.

This entire for loop could be skipped in case !eval.validate && !eval.generate holds true.
Please take a close look, but if this true, I think that we could extract this for-loop into a separate method. That would address the indentation issue, provide faster code and work for the indexer - all at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, thanks!

dataNew := data.WithUpdatedRewards(eval.proto, rewardlvl)
effectiveMinBalance := dataNew.MinBalance(&eval.proto)
if dataNew.MicroAlgos.Raw < effectiveMinBalance.Raw {
return fmt.Errorf("transaction %v: account %v balance %d below min %d (%d assets)",
txid, addr, dataNew.MicroAlgos.Raw, effectiveMinBalance.Raw, len(dataNew.Assets))
}

// Check if we have exceeded the maximum minimum balance
if eval.proto.MaximumMinimumBalance != 0 {
if effectiveMinBalance.Raw > eval.proto.MaximumMinimumBalance {
return fmt.Errorf("transaction %v: account %v would use too much space after this transaction. Minimum balance requirements would be %d (greater than max %d)", txid, addr, effectiveMinBalance.Raw, eval.proto.MaximumMinimumBalance)
// Check if we have exceeded the maximum minimum balance
if eval.proto.MaximumMinimumBalance != 0 {
if effectiveMinBalance.Raw > eval.proto.MaximumMinimumBalance {
return fmt.Errorf("transaction %v: account %v would use too much space after this transaction. Minimum balance requirements would be %d (greater than max %d)", txid, addr, effectiveMinBalance.Raw, eval.proto.MaximumMinimumBalance)
}
}
}
}
Expand Down