Skip to content

Conversation

@jannotti
Copy link
Contributor

@jannotti jannotti commented Sep 22, 2025

Summary

Addresses #6386

Allows an update transaction to change the extra-program-pages and global schema for an app. If epp or schema is non-zero in an update, then they are both updated. Requiring a non-zero in at least one field ensures that existing code will not "stumble onto" this feature. Today, all updates must have 0 epp and 0 schemas. It does mean that the app cannot reduce its epp to 0 and reduce the entire global schema to 0, however. We could use a non-zero local schema field to signal the update, even though updating the local schema is impossible, but that seems gross.

The entire MBR for epp and global schema is moved to the Sender of such an update. That does not include the base MBR (0.1A) for the app creation. We call an account that is responsible for MBR on an app that it did not create the "renter" for that app. I am open to better naming. That field is zeroAddress when the original creator is still responsible for the MBR, or if it becomes responsible again, by updating the app. It is available in AVM with app_params_get AppRenter

The PR is still a draft until we have a few more tests.

Test Plan

Key tests are TestSchemaUpdates and TestExtraPagesUpdate in ledger/applications_test.go.

A goal test/example is in app-update.sh of e2e subs.

@jannotti jannotti changed the title Allow epp and schema change Txn: Allow epp and schema change Sep 22, 2025
@jannotti jannotti self-assigned this Sep 22, 2025
@jannotti jannotti marked this pull request as draft September 22, 2025 18:13
@jannotti jannotti force-pushed the allow-epp-and-schema-change branch from 76e533c to 939889d Compare September 22, 2025 18:34
@joe-p
Copy link
Contributor

joe-p commented Sep 23, 2025

The MBR for the app is moved to the Sender of such an update. Currently, that includes the base MBR for the app, but I think that's probably a mistake, we need to consider how that might cause bugs in code that assumes a creator has at least a 0.1MBR. And we we need to ensure that delting such an app doesn't try to bring creator's MBR below 0.

So you're thinking

  1. Alice creates an app -> Alice MBR increased
  2. Bob updates and increases epp -> Bob MBR increased, Alice MBR stays the same
  3. Charlie updates and increases epp again -> Bob MBR "refunded", Charlie MBR increased, Alice MBR stays the same
  4. App is deleted -> Charlie MBR "refunded", Alice MBR "refunded"

This would require a new app param so the updater's account can be known in the even of a delete, correct?

@jannotti
Copy link
Contributor Author

All yes, except here:

  1. Bob updates and increases epp -> Bob MBR increased, Alice MBR stays the same

After this update, it is likely that Alice gets some of her MBR back. If the original app had a non-zero epp, or a non-zero global schema, the MBR for that stuff all moves over to Bob.

This would require a new app param so the updater's account can be known in the even of a delete, correct?

Yes, for lack of a better term, I'll be calling it "Renter". At app creation it is omitted (logically, that means the Renter is the creator). Therefore the entire MBR counts against the creator. In an update that changes sizes, the renter is set to the transaction sender, and MBR for epp and schema is moved (entirely).

This will create a new TEAL field for app_params_get - AppRenter. Should it return the creator when unset? Or return the zeroAddress? Returning the zeroAddress is probably more in keeping with how we handle AuthAddr.

@joe-p
Copy link
Contributor

joe-p commented Sep 23, 2025

Alice gets some of her MBR back

And that "some" is to ensure her account has enough MBR to stay in the ledger?

In other words, app creators must have 0.1 MBR and the latest account that increased the epp/schema must cover the MBR. Would the creator's 0.1 MBR count towards the app?

For example, total app MBR is 0.2 after Bob does an update with an increased epp.

A) Alice covers 0.1 and Bob covers 0.1

or

B) Bob covers the full 0.2 MBR, but Alice must also keep 0.1 MBR since she's the creator

@jannotti
Copy link
Contributor Author

Alice continues to be responsible for the basic 0.1A required for creating an app. But she's no longer responsible for any of the excess that is added for epp or global schema. (And, of course, still the 0.1 basic account MBR.)

In your example, Alice covers 0.1 and Bob covers 0.1.

@joe-p
Copy link
Contributor

joe-p commented Sep 23, 2025

Ok so

Static MBR: 100,000
Dynamic MBR: 100,000*(ExtraProgramPages) + (25,000+3,500)*schema.NumUint + (25,000+25,000)*schema.NumByteSlice

Static MBR stays with the creator. Dynamic MBR is paid by the renter. So if Alice creates 10 apps and Bob increases the epp on all of them, Alice's MBR is 1.0 ALGO

@jannotti
Copy link
Contributor Author

Alice's MBR is 1.0 ALGO

(plus 0.1 for the basic account MBR)

@jannotti jannotti force-pushed the allow-epp-and-schema-change branch 3 times, most recently from eb145de to beb1b6c Compare September 23, 2025 20:39
@algorand algorand deleted a comment from codecov bot Sep 23, 2025
@jannotti jannotti force-pushed the allow-epp-and-schema-change branch 2 times, most recently from a555e31 to 58e042f Compare September 24, 2025 15:47
@jannotti jannotti marked this pull request as ready for review September 24, 2025 18:53
@jannotti jannotti force-pushed the allow-epp-and-schema-change branch from 52f0f94 to 1e862f7 Compare September 25, 2025 14:51
@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

❌ Patch coverage is 45.83333% with 117 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@e68b54e). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
ledger/apply/application.go 31.50% 47 Missing and 3 partials ⚠️
cmd/goal/application.go 40.00% 18 Missing ⚠️
daemon/algod/api/server/v2/account.go 50.00% 7 Missing and 6 partials ⚠️
data/basics/teal.go 0.00% 8 Missing ⚠️
ledger/eval/appcow.go 55.55% 8 Missing ⚠️
data/txntest/txn.go 0.00% 5 Missing ⚠️
ledger/eval/eval.go 0.00% 5 Missing ⚠️
data/basics/testing/copiers.go 0.00% 4 Missing ⚠️
daemon/algod/api/server/v2/utils.go 66.66% 2 Missing and 1 partial ⚠️
daemon/algod/api/server/v2/dryrun.go 0.00% 2 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #6446   +/-   ##
=========================================
  Coverage          ?   46.16%           
=========================================
  Files             ?      662           
  Lines             ?   112095           
  Branches          ?        0           
=========================================
  Hits              ?    51747           
  Misses            ?    57585           
  Partials          ?     2763           

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

@SilentRhetoric
Copy link

May I suggest using the name "sponsor" for the account that takes over covering MBR for the app's EPP & global schema? I think this better aligns with the economic mechanism of MBR, which is a locked deposit rather than a series of payments over time.

@jannotti
Copy link
Contributor Author

May I suggest using the name "sponsor" for the account that takes over covering MBR for the app's EPP & global schema?

I do like that better than "renter". Any other ideas?

@jannotti jannotti force-pushed the allow-epp-and-schema-change branch from 1e862f7 to aab5b76 Compare October 2, 2025 20:10
@jannotti jannotti requested a review from algorandskiy October 3, 2025 01:10
@jannotti jannotti force-pushed the allow-epp-and-schema-change branch 2 times, most recently from 8ee020b to 232a50f Compare October 6, 2025 18:34
Still need to perform the changes to `counts` and `maxCounts`, as
described in the comments.
@jannotti jannotti force-pushed the allow-epp-and-schema-change branch from 232a50f to f3b5d85 Compare October 6, 2025 19:17
@jannotti jannotti requested review from cce and gmalouf October 6, 2025 19:52
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