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

Algod: Replace logic.DebuggerHook with logic.EvalTracer interface #4438

Merged
merged 139 commits into from
Jan 23, 2023

Conversation

jdtzmn
Copy link
Contributor

@jdtzmn jdtzmn commented Aug 19, 2022

Summary

This PR upgrades the existing logic.DebuggerHook interface to be more powerful & lightweight. To be specific:

  • Introduce new logic.EvalTracer interface and rename existing logic.DebuggerHook interface to logic.Debugger to be more intuitive.
  • New EvalTracer hooks are introduced for before/after transaction groups, before/after transactions, and after TEAL opcodes.
  • The TEAL evaluator will carry forward the debugger into any inner transaction evaluations.
  • The EvalContext.refreshDebugState function which used to be called before every TEAL op has been removed. This function copied and base64 encoded the entire stack and scratch space every time it was invoked, which is potentially expensive and not needed for the eventual simulation trace.

A new logic.MakeEvalTracerDebuggerAdaptor adaptor function has been created so that existing logic.Debugger implementors (e.g. tealdbg and the dryrun endpoint) can be ran as logic.EvalTracer with the same behavior.

What's Next

By itself this PR doesn't activate any new behavior. Instead, it defines the tracer hooks that will be necessary for the following:

  • Near term goal: return ApplyData information from simulated transactions
    • It's not necessary to use a tracer to access this info if the transactions succeed, however if a transaction fails, the block evaluator will not return the partially-completed ApplyData. To solve this without modifying the block evaluator, we can use a tracer to save a copy of the ApplyData after each operation. That's what the follow-up PR Algod: Additional simulation result information #4439 does.
  • Longer term goal: enable simulation trace collection

Test Plan

  • The old debugger tests still pass, now using logic.MakeEvalTracerDebuggerAdaptor
  • Test BlockEvaluator.TransactionGroup calls tracer hooks correctly
  • Test verify.TxnGroupWithDebugger calls tracer hooks correctly

jdtzmn added 30 commits July 20, 2022 17:18
Dynamic budget checks in the evaluator complicate the gatekeeping. This functionality will be easier to add in the future after the `DebuggerHook` interface has been extended.
This test confirms an error spotted by @jannotti where `errors.As(…)` was always true.

algorand#4322 (comment)
data/transactions/logic/debugger.go Outdated Show resolved Hide resolved
data/transactions/logic/debugger_test.go Outdated Show resolved Hide resolved
data/transactions/logic/debugger_test.go Show resolved Hide resolved
data/transactions/logic/eval.go Show resolved Hide resolved
data/transactions/logic/eval.go Show resolved Hide resolved
data/transactions/verify/txn_test.go Outdated Show resolved Hide resolved
data/txntest/txn.go Show resolved Hide resolved
ledger/internal/eval.go Outdated Show resolved Hide resolved
ledger/internal/eval_test.go Outdated Show resolved Hide resolved
ledger/simulation/simulator.go Show resolved Hide resolved
@jasonpaulos jasonpaulos deleted the branch algorand:master December 19, 2022 21:25
@jasonpaulos jasonpaulos reopened this Dec 19, 2022
@jasonpaulos jasonpaulos changed the base branch from feature/simulate-endpoint to master January 12, 2023 18:22
@jasonpaulos jasonpaulos changed the title Algod: Extend debugger hooks interface Algod: Replace logic.DebuggerHook with logic.EvalTracer interface Jan 17, 2023
data/transactions/logic/eval.go Show resolved Hide resolved
data/transactions/logic/mocktracer/tracer.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants