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

Enable debugging itxn programs #2900

Merged

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Sep 15, 2021

Summary

Set special addressed in EvalParams when evaluating from tealdbg/dryrun

Test Plan

Tests added

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2021

Codecov Report

Merging #2900 (1638429) into master (d1aca92) will increase coverage by 0.19%.
The diff coverage is 13.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2900      +/-   ##
==========================================
+ Coverage   47.10%   47.29%   +0.19%     
==========================================
  Files         349      355       +6     
  Lines       56351    57179     +828     
==========================================
+ Hits        26544    27043     +499     
- Misses      26831    27078     +247     
- Partials     2976     3058      +82     
Impacted Files Coverage Δ
cmd/algoh/main.go 0.00% <0.00%> (ø)
cmd/goal/account.go 14.32% <0.00%> (-0.46%) ⬇️
cmd/goal/accountsList.go 0.00% <0.00%> (ø)
cmd/goal/application.go 12.77% <0.00%> (-0.10%) ⬇️
cmd/goal/clerk.go 9.11% <0.00%> (-0.03%) ⬇️
cmd/goal/commands.go 10.81% <0.00%> (-0.26%) ⬇️
config/version.go 9.09% <ø> (ø)
crypto/secp256k1/panic_cb.go 0.00% <ø> (ø)
crypto/secp256k1/scalar_mult_cgo.go 77.77% <ø> (ø)
crypto/secp256k1/secp256.go 42.25% <ø> (ø)
... and 87 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1aca92...1638429. Read the comment docs.

@algorandskiy algorandskiy changed the title WIP: Enable debugging itxn programs Enable debugging itxn programs Sep 16, 2021
@algorandskiy algorandskiy marked this pull request as ready for review September 16, 2021 00:18
@@ -338,41 +338,6 @@ func (dl *dryrunLedger) GetCreatorForRound(rnd basics.Round, cidx basics.Creatab
return basics.Address{}, false, fmt.Errorf("unknown creatable type %d", ctype)
}

func (dl *dryrunLedger) getAppParams(addr basics.Address, aidx basics.AppIndex) (params basics.AppParams, err error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed some unused code

@algorandskiy algorandskiy self-assigned this Sep 16, 2021
@algorandskiy algorandskiy changed the base branch from master to rel/beta September 16, 2021 00:31
@algojack algojack changed the base branch from rel/beta to master September 16, 2021 16:13
var response generated.DryrunResponse
doDryrunRequest(&dr, &response)
require.NoError(t, err)
checkAppCallPass(t, &response)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like we neglected to add inner transactions to the DryrunTxnResult API type. @jannotti was there a reason for this, or did we just forgot about it?

Ideally any created inner transactions would be returned in that response and this test could verify they are there and properly formatted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely not on purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am incapable of expressing how much I hate that dryrun lives in its own universe where we have to do everything all over again.

@jannotti jannotti merged commit 4b67562 into algorand:master Sep 16, 2021
algobarb pushed a commit that referenced this pull request Sep 17, 2021
@algobarb algobarb mentioned this pull request Sep 17, 2021
tsachiherman pushed a commit that referenced this pull request Sep 17, 2021
![GitHub Logo](https://raw.githubusercontent.com/algorand/go-algorand/master/release/release-banner.jpg)




# Highlights

This version adds debugging for itxn programs and includes a bugfix for dryrun crashes. 
**This release does not contain a protocol upgrade.**

# Changes

1. TEAL
    * Added
        * Enable debugging itxn programs (#2900)
    * Fixed
        * Fix dryrun crash on rewards calculation (#2894)

## Additional Resources
* [Algorand Forum](https://forum.algorand.org)
* [Developer Documentation](https://developer.algorand.org)
@algorandskiy algorandskiy deleted the pavel/tealdbg-itxn-submit-fix branch September 21, 2021 16:55
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.

4 participants