-
Notifications
You must be signed in to change notification settings - Fork 523
Add application account into a dryrun req created by goal #2945
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
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
jasonpaulos
left a comment
There was a problem hiding this 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...) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
Summary
goal clerkneeds to consider this account when creates dryrun request object--dryrun-accountsfor 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)