Upgrade Market contract to put swap behind allowlist role -- Mumbai and Polygon

[amiecorso]

Upgrade artifacts for both networks.

Mumbai has more artifacts than Polygon because I accidentally used the "market" tag (lowercase) instead of the "Market" tag, which deploys all contracts in the market network. On Polygon I used the right tag to be more specific.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (885ae03) 64.1% compared to head (61656dd) 64.1%.

Additional details and impacted files

@@          Coverage Diff           @@
##           master    #727   +/-   ##
======================================
  Coverage    64.1%   64.1%           
======================================
  Files          14      14           
  Lines         980     980           
  Branches      152     152           
======================================
  Hits          629     629           
  Misses        292     292           
  Partials       59      59           

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

[jaycenhorton]

why did a previous deployment data change?

[amiecorso]

whoa I have no idea -- any theories? I did run all of these with slightly different compiler optimization settings because of the size of the market contract, which I think explains why all of the market contract members were indeed upgraded when no source code changed except for the Market's... but that doesn't explain why anything should affect an old deployment.

[jaycenhorton]

no idea, but it's pretty alarming (not so much for the changes it made here, but in that it can happen otherwise in the future on mainnet contracts)

[amiecorso]

so I tried to formulate a clear question for chatGPT, and this is what it had to say. obviously could be super wrong, but I could also see this as compelling? (there was a compiler settings update during this upgrade) Couldn't find anything online that felt helpful short of doing a full deep dive on how this plugin works and how all of these artifacts are managed:

ChatGPT:
"The change you observed in your .openzeppelin/polygon-mumbai.json file, specifically in the "type" field of a contract's storage layout, is intriguing and can be explained by understanding a few aspects of upgradeable smart contracts, storage layouts, and how OpenZeppelin manages these.

Understanding Storage Layouts in Upgradeable Contracts
Storage Layout Stability: In upgradeable contracts, it's crucial to maintain a consistent storage layout between different versions. This means that variables should be added to the end of the storage layout, and existing variables shouldn't be removed or reordered. Any change in the storage layout can lead to storage collisions and unintended behavior.

Type Field in Storage Layout: The type field in the storage layout describes the data type of a particular storage slot. For example, t_mapping(t_uint256,t_struct(Schedule)18070_storage) indicates a mapping from uint256 to a Schedule struct. The number (18070 or 18088) likely represents an internal identifier or versioning used by OpenZeppelin or the Solidity compiler for the struct type.

Possible Reasons for the Observed Change
Compiler Update or Configuration Change: If you updated your Solidity compiler or changed any compiler settings, the internal representation of struct types could have changed. This would result in a different identifier for the struct in the storage layout.

Struct Definition Change: If there were any modifications (even seemingly minor ones) to the Schedule struct itself or any struct that Schedule depends on, this could result in a new internal identifier being assigned.

OpenZeppelin Tooling Updates: If you updated OpenZeppelin's tools or libraries, they might handle the generation of these identifiers differently, leading to the change you observed.

Significance of "18088_storage"
The 18088_storage in the type field seems to be a unique identifier or version number for the Schedule struct as understood by the Solidity compiler or OpenZeppelin's tooling. This change indicates a different internal representation of the Schedule struct.
Additional Considerations
Immutability of Deployed Code: You are correct that the code on the blockchain is immutable. However, the representation of this code in your local artifacts (like the .openzeppelin folder) can change based on tooling and compiler versions.

Artifact Management: It's important to track changes in these artifacts, especially in the context of upgradeable contracts. Ensuring that the storage layout remains compatible between contract upgrades is critical for avoiding issues.

Version Control and Documentation: Keeping a version-controlled history of these files and documenting any changes in the contract's structure, compiler versions, and tooling can help in understanding and managing these changes.

In summary, the change you observed is likely due to a modification in how the Schedule struct is internally represented, possibly triggered by changes in the compiler, struct definition, or OpenZeppelin tooling. It's important to track such changes carefully to ensure compatibility and stability in upgradeable smart contracts."

[amiecorso]

oh and what else is interesting -- if you walk back through the commit history in this file I actually find evidence of these types of identifier changes all over the place