Skip to content

Conversation

@jannotti
Copy link
Contributor

@jannotti jannotti commented Sep 16, 2025

Summary

This change makes it clearer that there is no sharing among the storageDeltas. There was no sharing, because each pointed to object was freshly allocated, but using a pointer obscured that fact (and performed a wasted allocation.

This change will make it easier (or at least clearer) to make changes to maxCounts if we allow transactions that update an app's global schema.

Test Plan

It's makes it clearer that there is no sharing among the
storageDeltas. There was no sharing, because each pointed to object
was freshly allocated, but using a pointer obscured that fact (and
performed a wasted allocation.

This change will make it easier (or at least clearer) to make changes
to `maxCounts` if we allow transactions that update an app's global
schema.
@jannotti jannotti self-assigned this Sep 16, 2025
@algorandskiy algorandskiy requested a review from cce September 16, 2025 18:55
@jannotti jannotti requested a review from gmalouf September 17, 2025 14:03
@cce cce requested a review from Copilot September 17, 2025 19:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies memory management by changing counts and maxCounts fields in storageDelta from pointers to simple values, eliminating unnecessary pointer allocations while clarifying that no sharing occurs between storage deltas.

  • Changed counts and maxCounts fields from *basics.StateSchema to basics.StateSchema
  • Updated all assignments and comparisons to work with value types instead of pointers
  • Improved test assertions using require.ErrorContains for better readability

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ledger/eval/appcow.go Modified storageDelta struct to use value types and updated all field assignments
ledger/eval/appcow_test.go Updated test code to work with value types and improved error assertions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jannotti jannotti merged commit 7804433 into algorand:master Sep 17, 2025
41 checks passed
algorandskiy pushed a commit to algorandskiy/go-algorand that referenced this pull request Sep 25, 2025
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.

3 participants