Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: use proxy bin with storage slots in a namespace #6429

Merged
merged 3 commits into from
Aug 19, 2024

Conversation

kayagokalp
Copy link
Member

@kayagokalp kayagokalp commented Aug 16, 2024

Description

Changed the proxy contract such that storage slots are namespaced, to prevent accidental storage slot collision for proxy_owner field:

Storage decleartion is changed from:

storage {
        /// The [ContractId] of the target contract.
        ///
        /// # Additional Information
        ///
        /// `target` is stored at sha256("storage_SRC14_0")
        target in 0x7bb458adc1d118713319a5baa00a2d049dd64d2916477d2688d76970c898cd55: Option<ContractId> = None,
        /// The [State] of the proxy owner.
        ///
        /// # Additional Information
        proxy_owner: State = State::Uninitialized,
}

to:

storage {
    SRC14 {
        /// The [ContractId] of the target contract.
        ///
        /// # Additional Information
        ///
        /// `target` is stored at sha256("storage_SRC14_0")
        target in 0x7bb458adc1d118713319a5baa00a2d049dd64d2916477d2688d76970c898cd55: Option<ContractId> = None,
        /// The [State] of the proxy owner.
        ///
        /// # Additional Information
        proxy_owner: State = State::Uninitialized,
    },
}

@kayagokalp kayagokalp self-assigned this Aug 16, 2024
@kayagokalp kayagokalp added enhancement New feature or request forc-deploy Everything to do with forc-deploy labels Aug 16, 2024
sdankel
sdankel previously approved these changes Aug 16, 2024
@kayagokalp
Copy link
Member Author

Looks like i forgot to update contract id of the proxy contract in my previous commit. Now this should be good to go!

@kayagokalp kayagokalp requested a review from sdankel August 16, 2024 20:43
@kayagokalp kayagokalp mentioned this pull request Aug 17, 2024
8 tasks
@kayagokalp kayagokalp marked this pull request as ready for review August 17, 2024 16:09
@kayagokalp kayagokalp requested a review from a team as a code owner August 17, 2024 16:09
@JoshuaBatty JoshuaBatty requested a review from a team August 17, 2024 23:09
@kayagokalp kayagokalp enabled auto-merge (squash) August 17, 2024 23:45
Copy link
Contributor

@alfiedotwtf alfiedotwtf left a comment

Choose a reason for hiding this comment

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

I'm not familiar with the offset values, but LGTM otherwise

@kayagokalp kayagokalp merged commit 749e897 into master Aug 19, 2024
38 checks passed
@kayagokalp kayagokalp deleted the kayagokalp/move-namespaced-storage branch August 19, 2024 02:53
@kayagokalp
Copy link
Member Author

I'm not familiar with the offset values, but LGTM otherwise

Just to give context these are auto generated by forc. We just updated the contract that generates them

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request forc-deploy Everything to do with forc-deploy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants