Skip to content

Commit

Permalink
Upgradeability: use specific storage slot for owner functionality (#290)
Browse files Browse the repository at this point in the history
## Type of change

- Improvement (refactoring, restructuring repository, cleaning tech
debt, ...)

## Changes

The following changes have been made:

- The upgradeability lib has been updated to use a specific storage slot
for the proxy owner
- The upgradeability example has been updated to use a specific storage
slot for the proxy owner
- The upgradeability test contract has been updated to use a specific
storage slot for the proxy owner

## Notes

The `_proxy_owner()`, `only_proxy_owner()` and `_set_proxy_owner()`
functions no longer take `storage.proxy_owner` as a parameter. Instead
they directly read and write to the storage slot
`0xbb79927b15d9259ea316f2ecb2297d6cc8851888a98278c0a2e03e1a091ea754`
which is `sha256("storage_SRC14_1")`.

## Related Issues

<!--Delete everything after the "#" symbol and replace it with a number.
No spaces between hash and number-->

Closes #287 

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
- [x] I have updated the changelog to reflect the changes on this PR.

---------

Co-authored-by: bitzoic <[email protected]>
  • Loading branch information
K1-R1 and bitzoic authored Aug 30, 2024
1 parent fda3ebc commit d67f94c
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 47 deletions.
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [#286](https://github.com/FuelLabs/sway-libs/pull/286) `_set_metadata()` now reverts if the metadata is empty bytes.
- [#286](https://github.com/FuelLabs/sway-libs/pull/286) `_mint()` and `_burn()` now revert if the `amount` argument is zero.
- [#289](https://github.com/FuelLabs/sway-libs/pull/289) Bumps Sway-Libs to forc `v0.63.3`, fuel-core `v0.34.0`, and fuels `v0.66.2`.
- [#290](https://github.com/FuelLabs/sway-libs/pull/290) Update the Upgradeability library to use a specific storage slot for owner functionality.

### Fixed

Expand Down Expand Up @@ -144,6 +145,28 @@ fn foo(asset: AssetId, recipient: Identity, amount: u64, metadata: Metadata, key
}
```

- [#290](https://github.com/FuelLabs/sway-libs/pull/290) The `_proxy_owner()`, `only_proxy_owner()` and `_set_proxy_owner()` functions no longer take `storage.proxy_owner` as a parameter. Instead they directly read and write to the storage slot `0xbb79927b15d9259ea316f2ecb2297d6cc8851888a98278c0a2e03e1a091ea754` which is `sha256("storage_SRC14_1")`.

Before:

```sway
fn foo() {
let stored_proxy_owner = _proxy_owner(storage.proxy_owner);
only_proxy_owner(storage.proxy_owner);
_set_proxy_owner(new_proxy_owner, storage.proxy_owner);
}
```

After:

```sway
fn foo() {
let stored_proxy_owner = _proxy_owner();
only_proxy_owner();
_set_proxy_owner(new_proxy_owner);
}
```

## [v0.23.1]

### Added v0.23.1
Expand Down
26 changes: 18 additions & 8 deletions examples/upgradability/src/main.sw
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,21 @@ use standards::{src14::*, src5::*};
use sway_libs::upgradability::{_proxy_owner, _proxy_target, _set_proxy_target};
use standards::{src14::{SRC14, SRC14Extension}, src5::State};

#[namespace(SRC14)]
storage {
// target is at sha256("storage_SRC14_0")
target: Option<ContractId> = None,
proxy_owner: State = State::Uninitialized,
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` is stored at sha256("storage_SRC14_1")
proxy_owner in 0xbb79927b15d9259ea316f2ecb2297d6cc8851888a98278c0a2e03e1a091ea754: State = State::Uninitialized,
},
}

impl SRC14 for Contract {
Expand All @@ -31,7 +41,7 @@ impl SRC14 for Contract {
impl SRC14Extension for Contract {
#[storage(read)]
fn proxy_owner() -> State {
_proxy_owner(storage.proxy_owner)
_proxy_owner()
}
}
// ANCHOR_END: integrate_with_src14
Expand All @@ -53,21 +63,21 @@ fn proxy_target() -> Option<ContractId> {
// ANCHOR: set_proxy_owner
#[storage(write)]
fn set_proxy_owner(new_proxy_owner: State) {
_set_proxy_owner(new_proxy_owner, storage.proxy_owner);
_set_proxy_owner(new_proxy_owner);
}
// ANCHOR_END: set_proxy_owner

// ANCHOR: proxy_owner
#[storage(read)]
fn proxy_owner() -> State {
_proxy_owner(storage.proxy_owner)
_proxy_owner()
}
// ANCHOR_END: proxy_owner

// ANCHOR: only_proxy_owner
#[storage(read)]
fn only_proxy_owner_may_call() {
only_proxy_owner(storage.proxy_owner);
only_proxy_owner();
// Only the proxy's owner may reach this line.
}
// ANCHOR_END: only_proxy_owner
55 changes: 20 additions & 35 deletions libs/src/upgradability.sw
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ use ::upgradability::{errors::SetProxyOwnerError, events::{ProxyOwnerSet, ProxyT
use std::{auth::msg_sender, storage::storage_api::{read, write}};
use standards::{src14::SRC14_TARGET_STORAGE, src5::{AccessError, State}};

/// The storage slot to store the proxy owner State.
///
/// Value is `sha256("storage_SRC14_1")`.
pub const PROXY_OWNER_STORAGE: b256 = 0xbb79927b15d9259ea316f2ecb2297d6cc8851888a98278c0a2e03e1a091ea754;

/// Returns the proxy target.
///
/// # Returns
Expand Down Expand Up @@ -66,10 +71,6 @@ pub fn _set_proxy_target(new_target: ContractId) {

/// Returns the owner of the proxy.
///
/// # Arguments
///
/// * `proxy_owner_storage_key`: [StorageKey<State>] - The storage key of the stored proxy owner.
///
/// # Returns
///
/// * [State] - The state of the proxy ownership.
Expand All @@ -83,25 +84,19 @@ pub fn _set_proxy_target(new_target: ContractId) {
/// ```sway
/// use sway_libs::upgradability::_proxy_owner;
///
/// storage {
/// proxy_owner: State = State::Uninitialized,
/// }
///
/// fn foo() {
/// let stored_proxy_owner = _proxy_owner(storage.proxy_owner);
/// let stored_proxy_owner = _proxy_owner();
/// }
/// ```
#[storage(read)]
pub fn _proxy_owner(proxy_owner_storage_key: StorageKey<State>) -> State {
proxy_owner_storage_key.read()
pub fn _proxy_owner() -> State {
let proxy_owner_key = StorageKey::new(PROXY_OWNER_STORAGE, 0, PROXY_OWNER_STORAGE);
proxy_owner_key.read()
}

/// Ensures that the sender is the proxy owner.
///
/// # Arguments
///
/// * `proxy_owner_storage_key`: [StorageKey<State>] - The storage key of the stored proxy owner.
///
/// # Reverts
///
/// * When the sender is not the proxy owner.
Expand All @@ -115,19 +110,15 @@ pub fn _proxy_owner(proxy_owner_storage_key: StorageKey<State>) -> State {
/// ```sway
/// use sway_libs::ownership::only_proxy_owner;
///
/// storage {
/// proxy_owner: State = State::Uninitialized,
/// }
///
/// fn foo() {
/// only_proxy_owner(storage.proxy_owner);
/// only_proxy_owner();
/// // Do stuff here if the sender is the proxy owner
/// }
/// ```
#[storage(read)]
pub fn only_proxy_owner(proxy_owner_storage_key: StorageKey<State>) {
pub fn only_proxy_owner() {
require(
_proxy_owner(proxy_owner_storage_key) == State::Initialized(msg_sender().unwrap()),
_proxy_owner() == State::Initialized(msg_sender().unwrap()),
AccessError::NotOwner,
);
}
Expand All @@ -141,7 +132,6 @@ pub fn only_proxy_owner(proxy_owner_storage_key: StorageKey<State>) {
/// # Arguments
///
/// * `new_proxy_owner`: [State] - The new state of the proxy ownership.
/// * `proxy_owner_storage_key`: [StorageKey<State>] - The storage key of the stored proxy owner.
///
/// # Reverts
///
Expand All @@ -157,31 +147,26 @@ pub fn only_proxy_owner(proxy_owner_storage_key: StorageKey<State>) {
/// ```sway
/// use sway_libs::upgradability::{_proxy_owner, _set_proxy_owner};
///
/// storage {
/// proxy_owner: State = State::Uninitialized,
/// }
///
/// fn foo(new_owner: Identity) {
/// assert(_proxy_owner(storage.proxy_owner) == State::Initialized(Identity::Address(Address::zero()));
/// assert(_proxy_owner() == State::Initialized(Identity::Address(Address::zero()));
///
/// let new_proxy_owner = State::Initialized(new_owner);
/// _set_proxy_owner(new_proxy_owner, storage.proxy_owner);
/// _set_proxy_owner(new_proxy_owner);
///
/// assert(_proxy_owner(storage.proxy_owner) == State::Initialized(new_owner));
/// assert(_proxy_owner() == State::Initialized(new_owner));
/// }
/// ```
#[storage(write)]
pub fn _set_proxy_owner(
new_proxy_owner: State,
proxy_owner_storage_key: StorageKey<State>,
) {
only_proxy_owner(proxy_owner_storage_key);
pub fn _set_proxy_owner(new_proxy_owner: State) {
only_proxy_owner();

require(
new_proxy_owner != State::Uninitialized,
SetProxyOwnerError::CannotUninitialize,
);

proxy_owner_storage_key.write(new_proxy_owner);
let proxy_owner_key = StorageKey::new(PROXY_OWNER_STORAGE, 0, PROXY_OWNER_STORAGE);
proxy_owner_key.write(new_proxy_owner);

log(ProxyOwnerSet {
new_proxy_owner,
Expand Down
12 changes: 8 additions & 4 deletions tests/src/upgradability/src/main.sw
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ storage {
/// `target` is stored at sha256("storage_SRC14_0")
target in 0x7bb458adc1d118713319a5baa00a2d049dd64d2916477d2688d76970c898cd55: Option<ContractId> = None,
/// The [State] of the proxy owner.
proxy_owner: State = State::Uninitialized,
///
/// # Additional Information
///
/// `proxy_owner` is stored at sha256("storage_SRC14_1")
proxy_owner in 0xbb79927b15d9259ea316f2ecb2297d6cc8851888a98278c0a2e03e1a091ea754: State = State::Uninitialized,
},
}

Expand Down Expand Up @@ -53,19 +57,19 @@ impl SRC14 for Contract {
impl SRC14Extension for Contract {
#[storage(read)]
fn proxy_owner() -> State {
_proxy_owner(storage::SRC14.proxy_owner)
_proxy_owner()
}
}

impl UpgradableTest for Contract {
#[storage(read)]
fn only_proxy_owner() {
only_proxy_owner(storage::SRC14.proxy_owner);
only_proxy_owner();
}

#[storage(write)]
fn set_proxy_owner(new_proxy_owner: State) {
_set_proxy_owner(new_proxy_owner, storage::SRC14.proxy_owner);
_set_proxy_owner(new_proxy_owner);
}

// Used to immediately set the storage variables as the configured constants
Expand Down

0 comments on commit d67f94c

Please sign in to comment.