Inner clearstate #3556
Inner clearstate #3556
Conversation
Should also implement the "CSP requires 700 budget and can't spend more than 700 budget" rule, but tests still required.
jannotti
force-pushed
the
inner-clearstate
branch
from
1cbe56e to
bd29bd8
Compare
jannotti
force-pushed
the
inner-clearstate
branch
from
2d2e2df to
e863765
Compare
jannotti
force-pushed
the
inner-clearstate
branch
from
e863765 to
ce12e59
Compare
data/transactions/logic/eval.go
Outdated
| } | ||
|
|
||
| func (e ClearStateBudgetError) Error() string { | ||
| return fmt.Sprintf("ClearState execution with only %d", e.offered) |
There was a problem hiding this comment.
nit: can the error message refer to PooledApplicationBudget or something like that?
There was a problem hiding this comment.
Changed:
return fmt.Sprintf("Attempted ClearState execution with low OpcodeBudget %d", e.offered)
That should call to mind the global OpcodeBudget which is the problem.
There was a problem hiding this comment.
Looks correct, I just have questions/comments on tests.
Additionally, I don't think there's any tests for the changes in clear state budget? One suggestion would be a test where an app calls an inner clear state that has an infinite loop and we verify the global OpcodeBudget only decreases by ~700. Another test I'd like to see is one that makes sure ClearStateBudgetError actually fails the txn.
Correct, I still need both tests. Going into csp with less than 700 should NOT clear, but should fail. A csp that tries to take 701 should fail, but not kill caller, and should clear state. |
Codecov Report
@@ Coverage Diff @@
## feature/contract-to-contract #3556 +/- ##
================================================================
+ Coverage 47.80% 47.88% +0.07%
================================================================
Files 370 370
Lines 59959 60019 +60
================================================================
+ Hits 28665 28740 +75
+ Misses 27994 27977 -17
- Partials 3300 3302 +2
Continue to review full report at Codecov.
|
jasonpaulos
left a comment
jasonpaulos
left a comment
There was a problem hiding this comment.
Changes look good to me
5 participants
Prevents inner txns and bad opcode budget use in clear state programs.