Skip to content

Commit 35302d2

Browse files
authored
feat: add back assoc. error type to IOwnable (#851)
<!-- Thank you for your interest in contributing to OpenZeppelin! Consider opening an issue for discussion prior to submitting a PR. New features will be merged faster if they were first discussed and designed with the team. Describe the changes introduced in this pull request. Include any context necessary for understanding the PR's purpose. --> <!-- Fill in with issue number --> Resolves #??? #### PR Checklist <!-- Before merging the pull request all of the following must be completed. Feel free to submit a PR or Draft PR even if some items are pending. Some of the items may not apply. --> - [x] Tests - [x] Documentation - [x] Changelog
1 parent f0939f3 commit 35302d2

File tree

10 files changed

+174
-55
lines changed

10 files changed

+174
-55
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- `IErc721Wrapper` returns `Vec<u8>` instead of typed `Error`. #822
1313
- Consolidated Solidity interfaces and standardized naming (`*Interface`, `*Abi`, `I*`). #829
14+
- Added back associated error type to `IOwnable` trait.
15+
- `IUpgradeableBeacon` now expects `IOwnable<Error = Vec<u8>>` instead of `IOwnable`.
1416

1517
## [v0.3.0] - 2025-09-10
1618

contracts/src/access/ownable.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ pub struct Ownable {
7575
/// Interface for an [`Ownable`] contract.
7676
#[interface_id]
7777
pub trait IOwnable {
78+
/// The error type associated to the trait implementation.
79+
type Error: Into<alloc::vec::Vec<u8>>;
80+
7881
/// Returns the address of the current owner.
7982
///
8083
/// # Arguments
@@ -98,8 +101,10 @@ pub trait IOwnable {
98101
/// # Events
99102
///
100103
/// * [`OwnershipTransferred`].
101-
fn transfer_ownership(&mut self, new_owner: Address)
102-
-> Result<(), Vec<u8>>;
104+
fn transfer_ownership(
105+
&mut self,
106+
new_owner: Address,
107+
) -> Result<(), Self::Error>;
103108

104109
/// Leaves the contract without owner. It will not be possible to call
105110
/// functions that require `only_owner`. Can only be called by the current
@@ -119,11 +124,11 @@ pub trait IOwnable {
119124
/// # Events
120125
///
121126
/// * [`OwnershipTransferred`].
122-
fn renounce_ownership(&mut self) -> Result<(), Vec<u8>>;
127+
fn renounce_ownership(&mut self) -> Result<(), Self::Error>;
123128
}
124129

125130
#[public]
126-
#[implements(IOwnable, IErc165)]
131+
#[implements(IOwnable<Error = Error>, IErc165)]
127132
impl Ownable {
128133
/// Constructor.
129134
///
@@ -149,19 +154,21 @@ impl Ownable {
149154

150155
#[public]
151156
impl IOwnable for Ownable {
157+
type Error = Error;
158+
152159
fn owner(&self) -> Address {
153160
self.owner()
154161
}
155162

156163
fn transfer_ownership(
157164
&mut self,
158165
new_owner: Address,
159-
) -> Result<(), Vec<u8>> {
160-
Ok(self.transfer_ownership(new_owner)?)
166+
) -> Result<(), Self::Error> {
167+
self.transfer_ownership(new_owner)
161168
}
162169

163-
fn renounce_ownership(&mut self) -> Result<(), Vec<u8>> {
164-
Ok(self.renounce_ownership()?)
170+
fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
171+
self.renounce_ownership()
165172
}
166173
}
167174

contracts/src/access/ownable_two_step.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,27 @@ impl IOwnable2Step for Ownable2Step {
202202
}
203203
}
204204

205+
// This is implemented so that [`Ownable2Step`] could be passed to functions
206+
// expecting [`ownable::IOwnable`].
207+
impl ownable::IOwnable for Ownable2Step {
208+
type Error = ownable::Error;
209+
210+
fn owner(&self) -> Address {
211+
IOwnable2Step::owner(self)
212+
}
213+
214+
fn transfer_ownership(
215+
&mut self,
216+
new_owner: Address,
217+
) -> Result<(), Self::Error> {
218+
IOwnable2Step::transfer_ownership(self, new_owner)
219+
}
220+
221+
fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
222+
IOwnable2Step::renounce_ownership(self)
223+
}
224+
}
225+
205226
impl Ownable2Step {
206227
/// Transfers ownership of the contract to a new account (`new_owner`) and
207228
/// sets [`Self::pending_owner`] to [`Address::ZERO`] to avoid situations
@@ -239,7 +260,6 @@ mod tests {
239260
use stylus_sdk::{alloy_primitives::Address, prelude::*};
240261

241262
use super::*;
242-
use crate::access::ownable::IOwnable;
243263

244264
unsafe impl TopLevelStorage for Ownable2Step {}
245265

@@ -452,9 +472,11 @@ mod tests {
452472
assert!(contract.sender(alice).supports_interface(
453473
<Ownable2Step as IOwnable2Step>::interface_id()
454474
));
455-
assert!(contract
456-
.sender(alice)
457-
.supports_interface(<Ownable as IOwnable>::interface_id()));
475+
assert!(
476+
contract.sender(alice).supports_interface(
477+
<Ownable as ownable::IOwnable>::interface_id()
478+
)
479+
);
458480
assert!(contract
459481
.sender(alice)
460482
.supports_interface(<Ownable2Step as IErc165>::interface_id()));

contracts/src/proxy/beacon/upgradeable.rs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl From<ownable::Error> for Error {
7373
/// upgrading the proxies that use this beacon.
7474
///
7575
/// [BeaconProxy]: super::BeaconProxy
76-
pub trait IUpgradeableBeacon: IBeacon + IOwnable {
76+
pub trait IUpgradeableBeacon: IBeacon + IOwnable<Error = Vec<u8>> {
7777
/// Upgrades the beacon to a new implementation.
7878
///
7979
/// # Arguments
@@ -108,7 +108,7 @@ pub struct UpgradeableBeacon {
108108
unsafe impl TopLevelStorage for UpgradeableBeacon {}
109109

110110
#[public]
111-
#[implements(IBeacon, IOwnable)]
111+
#[implements(IBeacon, IOwnable<Error = Error>)]
112112
impl UpgradeableBeacon {
113113
/// Sets the address of the initial implementation, and the initial owner
114114
/// who can upgrade the beacon.
@@ -203,18 +203,20 @@ impl IBeacon for UpgradeableBeacon {
203203

204204
#[public]
205205
impl IOwnable for UpgradeableBeacon {
206+
type Error = Error;
207+
206208
fn owner(&self) -> Address {
207209
self.ownable.owner()
208210
}
209211

210212
fn transfer_ownership(
211213
&mut self,
212214
new_owner: Address,
213-
) -> Result<(), Vec<u8>> {
215+
) -> Result<(), Self::Error> {
214216
Ok(self.ownable.transfer_ownership(new_owner)?)
215217
}
216218

217-
fn renounce_ownership(&mut self) -> Result<(), Vec<u8>> {
219+
fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
218220
Ok(self.ownable.renounce_ownership()?)
219221
}
220222
}
@@ -529,13 +531,13 @@ mod tests {
529531
);
530532

531533
// the error should be from the ownable contract.
532-
assert_eq!(
534+
assert!(matches!(
533535
err,
534-
Error::InvalidOwner(ownable::OwnableInvalidOwner {
535-
owner: Address::ZERO,
536-
})
537-
.encode()
538-
);
536+
Error::InvalidOwner(
537+
ownable::OwnableInvalidOwner {
538+
owner,
539+
}) if owner == Address::ZERO,
540+
));
539541

540542
// ownership should remain unchanged.
541543
let owner = beacon.sender(alice).owner();
@@ -558,13 +560,12 @@ mod tests {
558560
.transfer_ownership(charlie)
559561
.expect_err("should fail when called by non-owner");
560562

561-
assert_eq!(
563+
assert!(matches!(
562564
err,
563565
Error::UnauthorizedAccount(ownable::OwnableUnauthorizedAccount {
564-
account: bob,
565-
})
566-
.encode()
567-
);
566+
account,
567+
}) if account == bob
568+
));
568569

569570
// ownership should remain unchanged.
570571
let owner = beacon.sender(alice).owner();
@@ -618,13 +619,12 @@ mod tests {
618619
.renounce_ownership()
619620
.expect_err("should fail when called by non-owner");
620621

621-
assert_eq!(
622+
assert!(matches!(
622623
err,
623624
Error::UnauthorizedAccount(ownable::OwnableUnauthorizedAccount {
624-
account: bob,
625-
})
626-
.encode()
627-
);
625+
account,
626+
}) if account == bob
627+
));
628628

629629
// ownership should remain unchanged.
630630
let owner = beacon.sender(alice).owner();

docs/modules/ROOT/pages/beacon-proxy.adoc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,17 @@ impl IBeacon for MyUpgradeableBeacon {
137137
138138
#[public]
139139
impl IOwnable for MyUpgradeableBeacon {
140+
type Error = Vec<u8>;
141+
140142
fn owner(&self) -> Address {
141143
self.beacon.owner()
142144
}
143145
144-
fn transfer_ownership(&mut self, new_owner: Address) -> Result<(), Vec<u8>> {
146+
fn transfer_ownership(&mut self, new_owner: Address) -> Result<(), Self::Error> {
145147
self.beacon.transfer_ownership(new_owner)
146148
}
147149
148-
fn renounce_ownership(&mut self) -> Result<(), Vec<u8>> {
150+
fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
149151
self.beacon.renounce_ownership()
150152
}
151153
}

docs/modules/ROOT/pages/uups-proxy.adoc

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,79 @@ Minimal example with `Ownable`, `UUPSUpgradeable`, and `Erc20` logic:
7272

7373
[source,rust]
7474
----
75+
use openzeppelin_stylus::{
76+
access::ownable::{self, IOwnable, Ownable},
77+
proxy::{
78+
erc1967,
79+
utils::{
80+
erc1822::IErc1822Proxiable,
81+
uups_upgradeable::{self, IUUPSUpgradeable, UUPSUpgradeable},
82+
},
83+
},
84+
token::erc20::{self, Erc20, IErc20},
85+
utils::address,
86+
};
87+
88+
#[derive(SolidityError, Debug)]
89+
enum Error {
90+
UnauthorizedAccount(ownable::OwnableUnauthorizedAccount),
91+
InvalidOwner(ownable::OwnableInvalidOwner),
92+
UnauthorizedCallContext(uups_upgradeable::UUPSUnauthorizedCallContext),
93+
UnsupportedProxiableUUID(uups_upgradeable::UUPSUnsupportedProxiableUUID),
94+
InvalidImplementation(erc1967::utils::ERC1967InvalidImplementation),
95+
InvalidAdmin(erc1967::utils::ERC1967InvalidAdmin),
96+
InvalidBeacon(erc1967::utils::ERC1967InvalidBeacon),
97+
NonPayable(erc1967::utils::ERC1967NonPayable),
98+
EmptyCode(address::AddressEmptyCode),
99+
FailedCall(address::FailedCall),
100+
FailedCallWithReason(address::FailedCallWithReason),
101+
InvalidInitialization(uups_upgradeable::InvalidInitialization),
102+
InvalidVersion(uups_upgradeable::InvalidVersion),
103+
}
104+
105+
impl From<uups_upgradeable::Error> for Error {
106+
fn from(e: uups_upgradeable::Error) -> Self {
107+
match e {
108+
uups_upgradeable::Error::InvalidImplementation(e) => {
109+
Error::InvalidImplementation(e)
110+
}
111+
uups_upgradeable::Error::InvalidAdmin(e) => Error::InvalidAdmin(e),
112+
uups_upgradeable::Error::InvalidBeacon(e) => {
113+
Error::InvalidBeacon(e)
114+
}
115+
uups_upgradeable::Error::NonPayable(e) => Error::NonPayable(e),
116+
uups_upgradeable::Error::EmptyCode(e) => Error::EmptyCode(e),
117+
uups_upgradeable::Error::FailedCall(e) => Error::FailedCall(e),
118+
uups_upgradeable::Error::FailedCallWithReason(e) => {
119+
Error::FailedCallWithReason(e)
120+
}
121+
uups_upgradeable::Error::InvalidInitialization(e) => {
122+
Error::InvalidInitialization(e)
123+
}
124+
uups_upgradeable::Error::UnauthorizedCallContext(e) => {
125+
Error::UnauthorizedCallContext(e)
126+
}
127+
uups_upgradeable::Error::UnsupportedProxiableUUID(e) => {
128+
Error::UnsupportedProxiableUUID(e)
129+
}
130+
uups_upgradeable::Error::InvalidVersion(e) => {
131+
Error::InvalidVersion(e)
132+
}
133+
}
134+
}
135+
}
136+
137+
impl From<ownable::Error> for Error {
138+
fn from(e: ownable::Error) -> Self {
139+
match e {
140+
ownable::Error::UnauthorizedAccount(e) => {
141+
Error::UnauthorizedAccount(e)
142+
}
143+
ownable::Error::InvalidOwner(e) => Error::InvalidOwner(e),
144+
}
145+
}
146+
}
147+
75148
#[entrypoint]
76149
#[storage]
77150
struct MyUUPSContract {
@@ -81,7 +154,7 @@ struct MyUUPSContract {
81154
}
82155
83156
#[public]
84-
#[implements(IErc20<Error = erc20::Error>, IUUPSUpgradeable, IErc1822Proxiable, IOwnable)]
157+
#[implements(IErc20<Error = erc20::Error>, IUUPSUpgradeable, IErc1822Proxiable, IOwnable<Error = Error>)]
85158
impl MyUUPSContract {
86159
// Accepting owner here only to enable invoking functions directly on the
87160
// UUPS
@@ -148,6 +221,11 @@ A simple UUPS-compatible proxy using ERC-1967:
148221

149222
[source,rust]
150223
----
224+
use openzeppelin_stylus::proxy::{
225+
erc1967::{self, Erc1967Proxy},
226+
IProxy,
227+
};
228+
151229
#[entrypoint]
152230
#[storage]
153231
struct MyUUPSProxy {

examples/ownable/src/lib.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ struct OwnableExample {
5959
}
6060

6161
#[public]
62-
#[implements(IErc20<Error = Error>, IOwnable)]
62+
#[implements(IErc20<Error = Error>, IOwnable<Error = ownable::Error>)]
6363
impl OwnableExample {
6464
#[constructor]
6565
fn constructor(&mut self, initial_owner: Address) -> Result<(), Error> {
@@ -119,18 +119,20 @@ impl IErc20 for OwnableExample {
119119

120120
#[public]
121121
impl IOwnable for OwnableExample {
122+
type Error = ownable::Error;
123+
122124
fn owner(&self) -> Address {
123125
self.ownable.owner()
124126
}
125127

126128
fn transfer_ownership(
127129
&mut self,
128130
new_owner: Address,
129-
) -> Result<(), Vec<u8>> {
130-
Ok(self.ownable.transfer_ownership(new_owner)?)
131+
) -> Result<(), Self::Error> {
132+
self.ownable.transfer_ownership(new_owner)
131133
}
132134

133-
fn renounce_ownership(&mut self) -> Result<(), Vec<u8>> {
134-
Ok(self.ownable.renounce_ownership()?)
135+
fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
136+
self.ownable.renounce_ownership()
135137
}
136138
}

0 commit comments

Comments
 (0)