Skip to content

Conversation

@jannotti
Copy link
Contributor

@jannotti jannotti commented Nov 4, 2025

A localref inside of tx.Access ought to be able to use 0 for "current app". It was an oversight to prohibit it. This PR allows it, but is consensus gated.

Also removes two consensus flags that can be assumed true now because they are strictly more lenient.

Add, indeed this consensus flag can be removed once it has gone into effect.

Addresses #6449

Summary

Test Plan

@jannotti jannotti self-assigned this Nov 4, 2025
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 63.63636% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.43%. Comparing base (a48733d) to head (afa94b0).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
daemon/algod/api/server/v2/utils.go 0.00% 11 Missing ⚠️
daemon/algod/api/server/v2/handlers.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6483      +/-   ##
==========================================
- Coverage   47.58%   47.43%   -0.16%     
==========================================
  Files         666      659       -7     
  Lines       88478    88395      -83     
==========================================
- Hits        42103    41930     -173     
- Misses      43607    43692      +85     
- Partials     2768     2773       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jannotti jannotti force-pushed the allow-0-local-app-ref branch 5 times, most recently from be023d6 to 38589bf Compare November 5, 2025 19:05
@jannotti jannotti marked this pull request as ready for review November 5, 2025 19:31
algorandskiy
algorandskiy previously approved these changes Nov 7, 2025
gmalouf
gmalouf previously approved these changes Nov 10, 2025
A localref inside of tx.Access ought to be able to use 0 for "current
app". It was an oversight to prohibit it.  This PR allows it, but is
consensus gated.

This was a slightly bigger that chnage than expected (because the
access list must now be scanned at app create time for localrefs woth
0 apps, like boxes were). So a few tests are needed before this is
reviewable.

Also removes two consensus flags that can be assumed true now because
they are strictly more lenient.
@jannotti jannotti dismissed stale reviews from gmalouf and algorandskiy via afa94b0 November 10, 2025 22:08
@jannotti jannotti force-pushed the allow-0-local-app-ref branch from 38589bf to afa94b0 Compare November 10, 2025 22:08
@jannotti jannotti requested a review from cce November 11, 2025 23:54
@jannotti jannotti merged commit b210374 into algorand:master Nov 12, 2025
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants