Skip to content

Conversation

@algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Sep 24, 2021

Summary

  • AVM 1.0 introduces application accounts
  • goal clerk needs to consider this account when creates dryrun request object
  • Added a new option --dryrun-accounts for adding accounts that were rekeyed to an app account (or any other accounts).

Test Plan

Tested manually. I'll add example from Jason W into sub-e2e tests in subseq PRs (need inner txns for indexer as well)

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2021

Codecov Report

Merging #2945 (03920c2) into master (ee0a715) will increase coverage by 0.01%.
The diff coverage is 3.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2945      +/-   ##
==========================================
+ Coverage   47.32%   47.33%   +0.01%     
==========================================
  Files         355      355              
  Lines       57230    57219      -11     
==========================================
+ Hits        27084    27085       +1     
+ Misses      27088    27071      -17     
- Partials     3058     3063       +5     
Impacted Files Coverage Δ
cmd/goal/application.go 13.46% <0.00%> (+0.68%) ⬆️
cmd/goal/commands.go 10.44% <0.00%> (-0.37%) ⬇️
libgoal/libgoal.go 3.03% <0.00%> (-0.01%) ⬇️
cmd/goal/clerk.go 9.18% <7.14%> (+0.06%) ⬆️
cmd/goal/common.go 75.00% <100.00%> (+0.92%) ⬆️
cmd/algoh/blockWatcher.go 77.77% <0.00%> (-3.18%) ⬇️
ledger/blockqueue.go 82.18% <0.00%> (-2.88%) ⬇️
util/metrics/gauge.go 68.00% <0.00%> (-2.67%) ⬇️
util/metrics/counter.go 78.57% <0.00%> (-2.39%) ⬇️
ledger/acctupdates.go 63.26% <0.00%> (+0.08%) ⬆️
... and 4 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 ee0a715...03920c2. Read the comment docs.

Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Mostly looks good, but I have some comments.

accounts = append(accounts, otherAccts...)

apps := []basics.AppIndex{tx.ApplicationID}
apps = append(apps, tx.ForeignApps...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not directly relevant to this change, but should this code also collect all foreign assets and look up the account balances of their creator to add to dr.Accounts?

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'll add this in a separate PR

@jannotti jannotti merged commit 073717e into algorand:master Sep 25, 2021
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