AVM: Share resource arrays across transactions #5035
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jannotti
force-pushed
the
avm-group-resources
branch
2 times, most recently
from
0971904
to
ac0b64f
Compare
Codecov Report
@@ Coverage Diff @@
## master #5035 +/- ##
==========================================
+ Coverage 53.77% 53.92% +0.14%
==========================================
Files 450 451 +1
Lines 56193 56401 +208
==========================================
+ Hits 30218 30414 +196
- Misses 23625 23636 +11
- Partials 2350 2351 +1
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
jannotti
force-pushed
the
avm-group-resources
branch
2 times, most recently
from
cdea9ff
to
72b3af4
Compare
jannotti
force-pushed
the
avm-group-resources
branch
from
9e4a99b
to
5fbe790
Compare
jasonpaulos
reviewed
Feb 14, 2023
jasonpaulos
reviewed
Feb 15, 2023
jannotti
force-pushed
the
avm-group-resources
branch
2 times, most recently
from
e56a06b
to
79a5f42
Compare
jasonpaulos
reviewed
Feb 16, 2023
jannotti
force-pushed
the
avm-group-resources
branch
from
c27d00d
to
f262a0e
Compare
jannotti
force-pushed
the
avm-group-resources
branch
from
8d30fca
to
8d0b6a9
Compare
jasonpaulos
reviewed
Feb 23, 2023
There was a problem hiding this comment.
Existing tests look good, I just left some minor comments.
Holding back on approving because I don't recall seeing any tests of a v9 app calling a v8 one, especially with regard to the strict foreign resource checks. It would be really nice to have coverage there.
ahangsu
reviewed
Apr 4, 2023
jasonpaulos
reviewed
Apr 4, 2023
bbroder-algo
reviewed
Apr 5, 2023
bbroder-algo
reviewed
Apr 5, 2023
bbroder-algo
reviewed
Apr 5, 2023
algorandskiy
previously approved these changes
Apr 5, 2023
I already have tests for the bug fix (the change is in EvalContext.availableAccount(), but they rely on a fair bit of code I have in the next PR, so I will include it there.
jasonpaulos
approved these changes
Apr 7, 2023
bbroder-algo
approved these changes
Apr 7, 2023
This was referenced May 9, 2023
This was referenced May 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will allow v9 apps to access resources named in any top-level transaction of the group, rather than only the resources named by this transaction. Non-appl transactions are treated as though they name of accounts and resources that are required to process the transaction.