Simulate: Add opcode costs and budgets into response #5221

algochoi · 2023-03-21T18:38:59Z

Summary

This PR adds the following support for the simulator:

reporting total opcode budgets and costs for app calls in a grouped transaction
reporting individual opcode costs for each top level transaction
- Inner app-call costs are "rolled-up" to the top level transaction
- Logic sig costs in inner transactions are not reported, since they cannot be pooled anyway
adds budget-added, budget-consumed fields in the algod API spec

Test Plan

Revised existing tests to report opcode budget and costs in the simulator and the handler. Added two new tests that tests over-budget app/logic sig calls to show behavior and how budget/costs are reported. Appreciate any other suggestions for tests as well 😄

codecov · 2023-03-21T18:57:59Z

Codecov Report

Merging #5221 (45363e5) into master (e659181) will decrease coverage by 7.02%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master    #5221      +/-   ##
==========================================
- Coverage   53.55%   46.53%   -7.02%     
==========================================
  Files         441      441              
  Lines       55123    55137      +14     
==========================================
- Hits        29520    25659    -3861     
- Misses      23317    27080    +3763     
- Partials     2286     2398     +112

Impacted Files	Coverage Δ
daemon/algod/api/server/v2/handlers.go	`0.82% <ø> (ø)`
daemon/algod/api/server/v2/utils.go	`10.91% <0.00%> (-0.26%)`	⬇️
ledger/simulation/trace.go	`78.94% <ø> (ø)`
ledger/simulation/simulator.go	`80.59% <100.00%> (-2.22%)`	⬇️
ledger/simulation/tracer.go	`91.54% <100.00%> (+0.92%)`	⬆️

... and 188 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

algochoi · 2023-03-22T18:16:31Z

ledger/simulation/tracer.go

 	}
 	tracer.cursorEvalTracer.BeforeTxnGroup(ep)
+
+	// Currently only supports one (first) txn group
+	if ep.PooledApplicationBudget != nil && tracer.result.TxnGroups[0].BudgetAdded == 0 {


We initialize the group budget by only accounting for top-level transactions. If we are in an inner level txn, then we dynamically add ep.Proto.MaxAppProgramCost to the budget each time. We might also want to think about how this can be handled when an app has depleted all its funds before it tries to "op-up" in an inner transaction and whether simulation should report an error in this case (like real execution), or simulate anyway.

when an app has depleted all its funds before it tries to "op-up" in an inner transaction and whether simulation should report an error in this case

That is what we discussed yesterday about keeping track of budget as execution goes, right?

I can understand that, the second if seems mutual exclusive with the first if, for first if condition makes BudgetAdded != 0, thus second if cannot be entered.

Could you explain more on what scenario the first if and second if will be triggered, such that it won't seem too abstract.

I forgot to clarify this in the previous comment:

I considered a case where ep.PooledApplicationBudget might be nil because of an old consensus version (before pooling was enabled, this pointer would just be nil). ~~Maybe for completion's sake, we should specify that the budget = ep.Proto.MaxAppProgramCost if this value is nil (since you cannot pool), and if it's not nil, we start adding the budget.~~

edit: I forgot there are also non-app calls that do not have a budget at all, so it's best to leave as is.

The BudgetAdded == 0 checks that we are always at the top-level transaction. If we are at an inner transaction, our PooledApplicationBudget could be something like 567 after being called midway through the app call. So we are saying that at the root-level transaction, set the BudgetAdded to a value that only accounts for the top level txns (e.g. 2100 for 3 txns) and then in the inner txn, we only add 700 once.

I think it's ok to do nothing here if EnableAppCostPooling is false (i.e. the current code looks good to me).

If I understand the code correctly, BudgetAdded for the group would then be 0 and get omitted from the response. This makes sense, since if app budgets aren't pooled, there's not much use in aggregating the budgets for the whole group anyway. And of course individual transactions' BudgetUsed would still be accurate in that case.

thanks for explaining! that helps me a lot in understanding and catching up!

ahangsu

taking a quick first pass, generally seems straightforward, have some minor questions as I am catching up.

ledger/simulation/simulator.go

ahangsu · 2023-03-22T19:41:16Z

ledger/simulation/tracer.go

 	}
 	tracer.cursorEvalTracer.BeforeTxnGroup(ep)
+
+	// Currently only supports one (first) txn group
+	if ep.PooledApplicationBudget != nil && tracer.result.TxnGroups[0].BudgetAdded == 0 {


when an app has depleted all its funds before it tries to "op-up" in an inner transaction and whether simulation should report an error in this case

That is what we discussed yesterday about keeping track of budget as execution goes, right?

ahangsu · 2023-03-22T19:43:26Z

ledger/simulation/tracer.go

 	}
 	tracer.cursorEvalTracer.BeforeTxnGroup(ep)
+
+	// Currently only supports one (first) txn group
+	if ep.PooledApplicationBudget != nil && tracer.result.TxnGroups[0].BudgetAdded == 0 {


I can understand that, the second if seems mutual exclusive with the first if, for first if condition makes BudgetAdded != 0, thus second if cannot be entered.

Could you explain more on what scenario the first if and second if will be triggered, such that it won't seem too abstract.

jasonpaulos

Looks great, I just have some minor comments and a test suggestion

daemon/algod/api/algod.oas2.json

daemon/algod/api/server/v2/utils.go

daemon/algod/api/algod.oas2.json

jasonpaulos · 2023-03-23T18:48:50Z

ledger/simulation/tracer.go

 	}
 	tracer.cursorEvalTracer.BeforeTxnGroup(ep)
+
+	// Currently only supports one (first) txn group
+	if ep.PooledApplicationBudget != nil && tracer.result.TxnGroups[0].BudgetAdded == 0 {


I think it's ok to do nothing here if EnableAppCostPooling is false (i.e. the current code looks good to me).

If I understand the code correctly, BudgetAdded for the group would then be 0 and get omitted from the response. This makes sense, since if app budgets aren't pooled, there's not much use in aggregating the budgets for the whole group anyway. And of course individual transactions' BudgetUsed would still be accurate in that case.

ledger/simulation/simulation_eval_test.go

jasonpaulos

Looks great, thanks for including logicsig budget as well!

daemon/algod/api/algod.oas3.yml

algochoi added 2 commits March 20, 2023 15:44

Report opcode budgets and fix simulate tests

8928c17

Fix inner txn costs

7b22cc9

algochoi added 3 commits March 22, 2023 11:20

Add opcode budget in algod handler

92d09cd

Merge branch 'master' into simulator-opcode-budget

41739f9

Finish resolving merge conflict

d81fd6d

algochoi added the Enhancement label Mar 22, 2023

algochoi added 2 commits March 22, 2023 11:53

Fix lint errors

7d9a9f4

Add test to show failed logicsig call and throw away budgets

3e27027

algochoi marked this pull request as ready for review March 22, 2023 17:57

algochoi requested review from ahangsu and jasonpaulos March 22, 2023 17:57

algochoi commented Mar 22, 2023

View reviewed changes

ahangsu reviewed Mar 22, 2023

View reviewed changes

Move assignment out of the loop

ceec16e

jasonpaulos reviewed Mar 23, 2023

View reviewed changes

algochoi added 5 commits March 23, 2023 15:54

Add Jason test and revisions in the API spec

a5cabc0

Remove uint64 casting in utils.go

41149fa

Add logic-sig cost fields - reconsider naming

9ad96fc

Rename budget variables

52e6872

Comment fixes

ef001b8

algochoi requested review from jasonpaulos and ahangsu March 24, 2023 14:27

jasonpaulos previously approved these changes Mar 24, 2023

View reviewed changes

algochoi requested a review from bbroder-algo March 24, 2023 17:54

ahangsu previously approved these changes Mar 24, 2023

View reviewed changes

algochoi added 2 commits March 27, 2023 12:34

Change app response field from budget-used to budget-consumed

e91425e

Merge branch 'master' into simulator-opcode-budget

45363e5

algochoi dismissed stale reviews from ahangsu and jasonpaulos via 45363e5 March 27, 2023 16:35

jasonpaulos approved these changes Mar 27, 2023

View reviewed changes

ahangsu approved these changes Mar 27, 2023

View reviewed changes

bbroder-algo approved these changes Mar 27, 2023

View reviewed changes

daemon/algod/api/algod.oas3.yml Outdated Show resolved Hide resolved

bbroder-algo merged commit 9a0f374 into algorand:master Mar 27, 2023

algochoi deleted the simulator-opcode-budget branch March 27, 2023 19:29

ahangsu mentioned this pull request Apr 4, 2023

Algod: Simulation run with increased limits on logs #5247

Merged

3 tasks

jasonpaulos mentioned this pull request Apr 26, 2023

Simulate: Make optional signatures an opt-in feature #5335

Merged

This was referenced May 9, 2023

go-algorand 3.16.0-beta Release PR #5370

Closed

go-algorand 3.16.0-beta Release PR #5371

Closed

go-algorand 3.16.0-beta Release PR #5382

Closed

Algo-devops-service mentioned this pull request May 22, 2023

go-algorand 3.16.0-beta Release PR #5406

Merged

This was referenced May 24, 2023

go-algorand 3.16.0-beta Release PR #5417

Merged

go-algorand 3.16.0-beta Release PR #5430

Merged

go-algorand 3.16.0-beta Release PR #5434

Merged

Algo-devops-service mentioned this pull request Jun 12, 2023

go-algorand 3.16.1-stable Release PR #5465

Merged

onetechnical mentioned this pull request Jun 14, 2023

go-algorand 3.16.2-stable Release PR #5469

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simulate: Add opcode costs and budgets into response #5221

Simulate: Add opcode costs and budgets into response #5221

algochoi commented Mar 21, 2023 •

edited

Loading

codecov bot commented Mar 21, 2023 •

edited

Loading

algochoi Mar 22, 2023

ahangsu Mar 22, 2023

ahangsu Mar 22, 2023

algochoi Mar 22, 2023 •

edited

Loading

jasonpaulos Mar 23, 2023

ahangsu Mar 24, 2023

ahangsu left a comment

ahangsu Mar 22, 2023

ahangsu Mar 22, 2023

jasonpaulos left a comment

jasonpaulos Mar 23, 2023

jasonpaulos left a comment

Simulate: Add opcode costs and budgets into response #5221

Simulate: Add opcode costs and budgets into response #5221

Conversation

algochoi commented Mar 21, 2023 • edited Loading

Summary

Test Plan

codecov bot commented Mar 21, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algochoi Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahangsu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonpaulos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonpaulos left a comment

Choose a reason for hiding this comment

algochoi commented Mar 21, 2023 •

edited

Loading

codecov bot commented Mar 21, 2023 •

edited

Loading

algochoi Mar 22, 2023 •

edited

Loading