diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0340f4d..a3e1671d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed (Breaking) +- All contract state fields are no longer public. #500 - Bump cargo-stylus to v0.5.8. #493 - Constants `TYPE_HASH`, `FIELDS`, `SALT` and `TYPED_DATA_PREFIX`, and type `DomainSeparatorTuple` are no longer exported from `utils::cryptography::eip712`. #478 - Bump Stylus SDK to v0.7.0. #433 diff --git a/contracts/src/access/control.rs b/contracts/src/access/control.rs index 6942a1d7a..4ae3e621f 100644 --- a/contracts/src/access/control.rs +++ b/contracts/src/access/control.rs @@ -118,8 +118,7 @@ pub struct RoleData { #[storage] pub struct AccessControl { /// Role identifier -> Role information. - #[allow(clippy::used_underscore_binding)] - pub _roles: StorageMap, RoleData>, + pub(crate) roles: StorageMap, RoleData>, } #[public] @@ -133,7 +132,7 @@ impl AccessControl { /// * `account` - The account to check for membership. #[must_use] pub fn has_role(&self, role: B256, account: Address) -> bool { - self._roles.getter(role).has_role.get(account) + self.roles.getter(role).has_role.get(account) } /// Checks if [`msg::sender`] has been granted `role`. @@ -162,7 +161,7 @@ impl AccessControl { /// * `role` - The role identifier. #[must_use] pub fn get_role_admin(&self, role: B256) -> B256 { - *self._roles.getter(role).admin_role + *self.roles.getter(role).admin_role } /// Grants `role` to `account`. @@ -291,7 +290,7 @@ impl AccessControl { /// Emits a [`RoleAdminChanged`] event. pub fn _set_role_admin(&mut self, role: B256, new_admin_role: B256) { let previous_admin_role = self.get_role_admin(role); - self._roles.setter(role).admin_role.set(new_admin_role); + self.roles.setter(role).admin_role.set(new_admin_role); evm::log(RoleAdminChanged { role, previous_admin_role, @@ -343,7 +342,7 @@ impl AccessControl { if self.has_role(role, account) { false } else { - self._roles.setter(role).has_role.insert(account, true); + self.roles.setter(role).has_role.insert(account, true); evm::log(RoleGranted { role, account, sender: msg::sender() }); true } @@ -365,7 +364,7 @@ impl AccessControl { /// May emit a [`RoleRevoked`] event. pub fn _revoke_role(&mut self, role: B256, account: Address) -> bool { if self.has_role(role, account) { - self._roles.setter(role).has_role.insert(account, false); + self.roles.setter(role).has_role.insert(account, false); evm::log(RoleRevoked { role, account, sender: msg::sender() }); true } else { @@ -404,11 +403,7 @@ mod tests { // NOTE: Once we have support for setting `msg::sender` and constructor, // this function shouldn't be needed. fn _grant_role_to_msg_sender(contract: &mut AccessControl, role: [u8; 32]) { - contract - ._roles - .setter(role.into()) - .has_role - .insert(msg::sender(), true); + contract.roles.setter(role.into()).has_role.insert(msg::sender(), true); } #[motsu::test] @@ -454,7 +449,7 @@ mod tests { #[motsu::test] fn admin_can_revoke_role(contract: AccessControl) { _grant_role_to_msg_sender(contract, AccessControl::DEFAULT_ADMIN_ROLE); - contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true); + contract.roles.setter(ROLE.into()).has_role.insert(ALICE, true); let has_role = contract.has_role(ROLE.into(), ALICE); assert!(has_role); @@ -465,7 +460,7 @@ mod tests { #[motsu::test] fn non_admin_cannot_revoke_role(contract: AccessControl) { - contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true); + contract.roles.setter(ROLE.into()).has_role.insert(ALICE, true); let has_role = contract.has_role(ROLE.into(), ALICE); assert!(has_role); @@ -536,7 +531,7 @@ mod tests { contract._set_role_admin(ROLE.into(), OTHER_ROLE.into()); _grant_role_to_msg_sender(contract, OTHER_ROLE); - contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true); + contract.roles.setter(ROLE.into()).has_role.insert(ALICE, true); contract.revoke_role(ROLE.into(), ALICE).unwrap(); let has_role = contract.has_role(ROLE.into(), ALICE); assert!(!has_role); @@ -583,14 +578,14 @@ mod tests { #[motsu::test] fn internal_grant_role_false_if_role(contract: AccessControl) { - contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true); + contract.roles.setter(ROLE.into()).has_role.insert(ALICE, true); let role_granted = contract._grant_role(ROLE.into(), ALICE); assert!(!role_granted); } #[motsu::test] fn internal_revoke_role_true_if_role(contract: AccessControl) { - contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true); + contract.roles.setter(ROLE.into()).has_role.insert(ALICE, true); let role_revoked = contract._revoke_role(ROLE.into(), ALICE); assert!(role_revoked); } diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs index 2ad282c53..e596b61be 100644 --- a/contracts/src/access/ownable.rs +++ b/contracts/src/access/ownable.rs @@ -69,8 +69,7 @@ impl MethodError for Error { #[storage] pub struct Ownable { /// The current owner of this contract. - #[allow(clippy::used_underscore_binding)] - pub _owner: StorageAddress, + pub(crate) owner: StorageAddress, } /// Interface for an [`Ownable`] contract. @@ -134,7 +133,7 @@ impl IOwnable for Ownable { type Error = Error; fn owner(&self) -> Address { - self._owner.get() + self.owner.get() } fn transfer_ownership( @@ -195,8 +194,8 @@ impl Ownable { /// /// Emits a [`OwnershipTransferred`] event. pub fn _transfer_ownership(&mut self, new_owner: Address) { - let previous_owner = self._owner.get(); - self._owner.set(new_owner); + let previous_owner = self.owner.get(); + self.owner.set(new_owner); evm::log(OwnershipTransferred { previous_owner, new_owner }); } } @@ -212,17 +211,17 @@ mod tests { #[motsu::test] fn reads_owner(contract: Ownable) { - contract._owner.set(msg::sender()); + contract.owner.set(msg::sender()); let owner = contract.owner(); assert_eq!(owner, msg::sender()); } #[motsu::test] fn transfers_ownership(contract: Ownable) { - contract._owner.set(msg::sender()); + contract.owner.set(msg::sender()); contract.transfer_ownership(ALICE).expect("should transfer ownership"); - let owner = contract._owner.get(); + let owner = contract.owner.get(); assert_eq!(owner, ALICE); } @@ -230,7 +229,7 @@ mod tests { fn prevents_non_onwers_from_transferring(contract: Ownable) { // Alice must be set as owner, because we can't set the // `msg::sender` yet. - contract._owner.set(ALICE); + contract.owner.set(ALICE); let bob = address!("B0B0cB49ec2e96DF5F5fFB081acaE66A2cBBc2e2"); let err = contract.transfer_ownership(bob).unwrap_err(); @@ -239,7 +238,7 @@ mod tests { #[motsu::test] fn prevents_reaching_stuck_state(contract: Ownable) { - contract._owner.set(msg::sender()); + contract.owner.set(msg::sender()); let err = contract.transfer_ownership(Address::ZERO).unwrap_err(); assert!(matches!(err, Error::InvalidOwner(_))); @@ -247,10 +246,10 @@ mod tests { #[motsu::test] fn loses_ownership_after_renouncing(contract: Ownable) { - contract._owner.set(msg::sender()); + contract.owner.set(msg::sender()); let _ = contract.renounce_ownership(); - let owner = contract._owner.get(); + let owner = contract.owner.get(); assert_eq!(owner, Address::ZERO); } @@ -258,7 +257,7 @@ mod tests { fn prevents_non_owners_from_renouncing(contract: Ownable) { // Alice must be set as owner, because we can't set the // `msg::sender` yet. - contract._owner.set(ALICE); + contract.owner.set(ALICE); let err = contract.renounce_ownership().unwrap_err(); assert!(matches!(err, Error::UnauthorizedAccount(_))); @@ -266,10 +265,10 @@ mod tests { #[motsu::test] fn recovers_access_using_internal_transfer(contract: Ownable) { - contract._owner.set(ALICE); + contract.owner.set(ALICE); contract._transfer_ownership(ALICE); - let owner = contract._owner.get(); + let owner = contract.owner.get(); assert_eq!(owner, ALICE); } } diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs index 8dca87d9f..6f09d7ce3 100644 --- a/contracts/src/access/ownable_two_step.rs +++ b/contracts/src/access/ownable_two_step.rs @@ -61,11 +61,11 @@ pub enum Error { #[storage] pub struct Ownable2Step { /// [`Ownable`] contract. - #[allow(clippy::used_underscore_binding)] - pub _ownable: Ownable, + // We leave the parent [`Ownable`] contract instance public, so that + // inheritting contract have access to its internal functions. + pub ownable: Ownable, /// Pending owner of the contract. - #[allow(clippy::used_underscore_binding)] - pub _pending_owner: StorageAddress, + pub(crate) pending_owner: StorageAddress, } /// Interface for an [`Ownable2Step`] contract. @@ -155,19 +155,19 @@ impl IOwnable2Step for Ownable2Step { type Error = Error; fn owner(&self) -> Address { - self._ownable.owner() + self.ownable.owner() } fn pending_owner(&self) -> Address { - self._pending_owner.get() + self.pending_owner.get() } fn transfer_ownership( &mut self, new_owner: Address, ) -> Result<(), Self::Error> { - self._ownable.only_owner()?; - self._pending_owner.set(new_owner); + self.ownable.only_owner()?; + self.pending_owner.set(new_owner); let current_owner = self.owner(); evm::log(OwnershipTransferStarted { @@ -191,7 +191,7 @@ impl IOwnable2Step for Ownable2Step { } fn renounce_ownership(&mut self) -> Result<(), Error> { - self._ownable.only_owner()?; + self.ownable.only_owner()?; self._transfer_ownership(Address::ZERO); Ok(()) } @@ -214,8 +214,8 @@ impl Ownable2Step { /// /// Emits a [`crate::access::ownable::OwnershipTransferred`] event. fn _transfer_ownership(&mut self, new_owner: Address) { - self._pending_owner.set(Address::ZERO); - self._ownable._transfer_ownership(new_owner); + self.pending_owner.set(Address::ZERO); + self.ownable._transfer_ownership(new_owner); } } @@ -231,33 +231,33 @@ mod tests { #[motsu::test] fn reads_owner(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); + contract.ownable.owner.set(msg::sender()); let owner = contract.owner(); assert_eq!(owner, msg::sender()); } #[motsu::test] fn reads_pending_owner(contract: Ownable2Step) { - contract._pending_owner.set(ALICE); + contract.pending_owner.set(ALICE); let pending_owner = contract.pending_owner(); assert_eq!(pending_owner, ALICE); } #[motsu::test] fn initiates_ownership_transfer(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); + contract.ownable.owner.set(msg::sender()); contract .transfer_ownership(ALICE) .expect("should initiate ownership transfer"); - let pending_owner = contract._pending_owner.get(); + let pending_owner = contract.pending_owner.get(); assert_eq!(pending_owner, ALICE); assert_eq!(contract.owner(), msg::sender()); } #[motsu::test] fn prevents_non_owners_from_initiating_transfer(contract: Ownable2Step) { - contract._ownable._owner.set(ALICE); + contract.ownable.owner.set(ALICE); let err = contract.transfer_ownership(BOB).unwrap_err(); assert!(matches!( @@ -268,8 +268,8 @@ mod tests { #[motsu::test] fn accepts_ownership(contract: Ownable2Step) { - contract._ownable._owner.set(ALICE); - contract._pending_owner.set(msg::sender()); + contract.ownable.owner.set(ALICE); + contract.pending_owner.set(msg::sender()); contract.accept_ownership().expect("should accept ownership"); assert_eq!(contract.owner(), msg::sender()); @@ -278,8 +278,8 @@ mod tests { #[motsu::test] fn prevents_non_pending_owner_from_accepting(contract: Ownable2Step) { - contract._ownable._owner.set(ALICE); - contract._pending_owner.set(BOB); + contract.ownable.owner.set(ALICE); + contract.pending_owner.set(BOB); let err = contract.accept_ownership().unwrap_err(); assert!(matches!( @@ -290,7 +290,7 @@ mod tests { #[motsu::test] fn completes_two_step_ownership_transfer(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); + contract.ownable.owner.set(msg::sender()); contract .transfer_ownership(ALICE) @@ -299,7 +299,7 @@ mod tests { // Simulate ALICE accepting ownership, since we cannot set `msg::sender` // in tests yet. - contract._pending_owner.set(msg::sender()); + contract.pending_owner.set(msg::sender()); contract.accept_ownership().expect("should accept ownership"); assert_eq!(contract.owner(), msg::sender()); @@ -308,7 +308,7 @@ mod tests { #[motsu::test] fn renounces_ownership(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); + contract.ownable.owner.set(msg::sender()); contract.renounce_ownership().expect("should renounce ownership"); assert_eq!(contract.owner(), Address::ZERO); @@ -316,7 +316,7 @@ mod tests { #[motsu::test] fn prevents_non_owners_from_renouncing(contract: Ownable2Step) { - contract._ownable._owner.set(ALICE); + contract.ownable.owner.set(ALICE); let err = contract.renounce_ownership().unwrap_err(); assert!(matches!( @@ -327,8 +327,8 @@ mod tests { #[motsu::test] fn cancels_transfer_on_renounce(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); - contract._pending_owner.set(ALICE); + contract.ownable.owner.set(msg::sender()); + contract.pending_owner.set(ALICE); contract.renounce_ownership().expect("should renounce ownership"); assert_eq!(contract.owner(), Address::ZERO); @@ -337,8 +337,8 @@ mod tests { #[motsu::test] fn allows_owner_to_cancel_transfer(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); - contract._pending_owner.set(ALICE); + contract.ownable.owner.set(msg::sender()); + contract.pending_owner.set(ALICE); contract .transfer_ownership(Address::ZERO) @@ -349,7 +349,7 @@ mod tests { #[motsu::test] fn allows_owner_to_overwrite_transfer(contract: Ownable2Step) { - contract._ownable._owner.set(msg::sender()); + contract.ownable.owner.set(msg::sender()); contract .transfer_ownership(ALICE) diff --git a/contracts/src/finance/vesting_wallet.rs b/contracts/src/finance/vesting_wallet.rs index 0e5f52656..e31a7d389 100644 --- a/contracts/src/finance/vesting_wallet.rs +++ b/contracts/src/finance/vesting_wallet.rs @@ -113,21 +113,19 @@ mod token { #[storage] pub struct VestingWallet { /// [`Ownable`] contract. + // We leave the parent [`Ownable`] contract instance public, so that + // inheritting contract have access to its internal functions. pub ownable: Ownable, /// Amount of Ether already released. - #[allow(clippy::used_underscore_binding)] - pub _released: StorageU256, + pub(crate) released: StorageU256, /// Amount of ERC-20 tokens already released. - #[allow(clippy::used_underscore_binding)] - pub _erc20_released: StorageMap, + pub(crate) erc20_released: StorageMap, /// Start timestamp. - #[allow(clippy::used_underscore_binding)] - pub _start: StorageU64, + pub(crate) start: StorageU64, /// Vesting duration. - #[allow(clippy::used_underscore_binding)] - pub _duration: StorageU64, + pub(crate) duration: StorageU64, /// [`SafeErc20`] contract. - pub safe_erc20: SafeErc20, + pub(crate) safe_erc20: SafeErc20, } /// NOTE: Implementation of [`TopLevelStorage`] to be able use `&mut self` when @@ -388,11 +386,11 @@ impl IVestingWallet for VestingWallet { fn receive_ether(&self) {} fn start(&self) -> U256 { - U256::from(self._start.get()) + U256::from(self.start.get()) } fn duration(&self) -> U256 { - U256::from(self._duration.get()) + U256::from(self.duration.get()) } fn end(&self) -> U256 { @@ -403,12 +401,12 @@ impl IVestingWallet for VestingWallet { #[selector(name = "released")] fn released_eth(&self) -> U256 { - self._released.get() + self.released.get() } #[selector(name = "released")] fn released_erc20(&self, token: Address) -> U256 { - self._erc20_released.get(token) + self.erc20_released.get(token) } #[selector(name = "releasable")] @@ -433,7 +431,7 @@ impl IVestingWallet for VestingWallet { fn release_eth(&mut self) -> Result<(), Self::Error> { let amount = self.releasable_eth(); - self._released.add_assign_checked( + self.released.add_assign_checked( amount, "total released should not exceed `U256::MAX`", ); @@ -452,7 +450,7 @@ impl IVestingWallet for VestingWallet { let amount = self.releasable_erc20(token)?; let owner = self.ownable.owner(); - self._erc20_released.setter(token).add_assign_checked( + self.erc20_released.setter(token).add_assign_checked( amount, "total released should not exceed `U256::MAX`", ); @@ -551,8 +549,8 @@ mod tests { ) -> (U64, U64) { let start = U64::from(start); let duration = U64::from(duration); - contract._start.set(start); - contract._duration.set(duration); + contract.start.set(start); + contract.duration.set(duration); (start, duration) } @@ -583,14 +581,14 @@ mod tests { #[motsu::test] fn reads_released_eth(contract: VestingWallet) { let one = uint!(1_U256); - contract._released.set(one); + contract.released.set(one); assert_eq!(one, contract.released_eth()); } #[motsu::test] fn reads_released_erc20(contract: VestingWallet) { let one = uint!(1_U256); - contract._erc20_released.setter(TOKEN).set(one); + contract.erc20_released.setter(TOKEN).set(one); assert_eq!(one, contract.released_erc20(TOKEN)); } diff --git a/contracts/src/lib.rs b/contracts/src/lib.rs index efca4003b..5a83ce0b4 100644 --- a/contracts/src/lib.rs +++ b/contracts/src/lib.rs @@ -38,11 +38,7 @@ impl MyContract { } ``` */ -#![allow( - clippy::pub_underscore_fields, - clippy::module_name_repetitions, - clippy::used_underscore_items -)] +#![allow(clippy::module_name_repetitions, clippy::used_underscore_items)] #![cfg_attr(not(feature = "std"), no_std, no_main)] #![cfg_attr(coverage_nightly, feature(coverage_attribute))] #![deny(rustdoc::broken_intra_doc_links)] diff --git a/contracts/src/token/erc1155/extensions/burnable.rs b/contracts/src/token/erc1155/extensions/burnable.rs index bae3789f6..97acabf12 100644 --- a/contracts/src/token/erc1155/extensions/burnable.rs +++ b/contracts/src/token/erc1155/extensions/burnable.rs @@ -165,7 +165,7 @@ mod tests { let initial_balance = contract.balance_of(BOB, token_ids[0]); assert_eq!(values[0], initial_balance); - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); contract .burn(BOB, token_ids[0], values[0]) @@ -200,7 +200,7 @@ mod tests { let invalid_sender = Address::ZERO; contract - ._operator_approvals + .operator_approvals .setter(invalid_sender) .setter(alice) .set(true); @@ -271,7 +271,7 @@ mod tests { assert_eq!(value, balance); } - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); contract .burn_batch(BOB, token_ids.clone(), values.clone()) @@ -308,7 +308,7 @@ mod tests { let invalid_sender = Address::ZERO; contract - ._operator_approvals + .operator_approvals .setter(invalid_sender) .setter(alice) .set(true); diff --git a/contracts/src/token/erc1155/extensions/metadata_uri.rs b/contracts/src/token/erc1155/extensions/metadata_uri.rs index e61d5ff35..ef72aafbf 100644 --- a/contracts/src/token/erc1155/extensions/metadata_uri.rs +++ b/contracts/src/token/erc1155/extensions/metadata_uri.rs @@ -36,8 +36,7 @@ mod sol { pub struct Erc1155MetadataUri { /// Used as the URI for all token types by relying on ID substitution, /// e.g. https://token-cdn-domain/{id}.json. - #[allow(clippy::used_underscore_binding)] - pub _uri: StorageString, + pub(crate) uri: StorageString, } /// Interface for the optional metadata functions from the ERC-1155 standard. @@ -61,7 +60,7 @@ impl IErc1155MetadataUri for Erc1155MetadataUri { /// Clients calling this function must replace the `id` substring with /// the actual token type ID. fn uri(&self, _id: U256) -> String { - self._uri.get_string() + self.uri.get_string() } } @@ -82,7 +81,7 @@ mod tests { #[motsu::test] fn uri_ignores_token_id(contract: Erc1155MetadataUri) { let uri = String::from("https://token-cdn-domain/\\{id\\}.json"); - contract._uri.set_str(uri.clone()); + contract.uri.set_str(uri.clone()); let token_id = uint!(1_U256); assert_eq!(uri, contract.uri(token_id)); diff --git a/contracts/src/token/erc1155/extensions/supply.rs b/contracts/src/token/erc1155/extensions/supply.rs index eb3e4f1ca..dca071681 100644 --- a/contracts/src/token/erc1155/extensions/supply.rs +++ b/contracts/src/token/erc1155/extensions/supply.rs @@ -1,7 +1,7 @@ //! Extension of ERC-1155 that adds tracking of total supply per token id. //! //! Useful for scenarios where Fungible and Non-fungible tokens have to be -//! clearly identified. Note: While a `_total_supply` of 1 might mean the +//! clearly identified. Note: While a `total_supply` of 1 might mean the //! corresponding is an NFT, there are no guarantees that no other tokens //! with the same id are not going to be minted. //! @@ -33,11 +33,9 @@ pub struct Erc1155Supply { /// ERC-1155 contract storage. pub erc1155: Erc1155, /// Mapping from token id to total supply. - #[allow(clippy::used_underscore_binding)] - pub _total_supply: StorageMap, + pub(crate) total_supply: StorageMap, /// Total supply of all token ids. - #[allow(clippy::used_underscore_binding)] - pub _total_supply_all: StorageU256, + pub(crate) total_supply_all: StorageU256, } /// Required interface of a [`Erc1155Supply`] contract. @@ -70,11 +68,11 @@ pub trait IErc1155Supply { impl IErc1155Supply for Erc1155Supply { fn total_supply(&self, id: U256) -> U256 { - self._total_supply.get(id) + self.total_supply.get(id) } fn total_supply_all(&self) -> U256 { - *self._total_supply_all + *self.total_supply_all } fn exists(&self, id: U256) -> bool { @@ -234,16 +232,16 @@ impl Erc1155Supply { if from.is_zero() { for (&token_id, &value) in token_ids.iter().zip(values.iter()) { - self._total_supply.setter(token_id).add_assign_checked( + self.total_supply.setter(token_id).add_assign_checked( value, - "should not exceed `U256::MAX` for `_total_supply`", + "should not exceed `U256::MAX` for `total_supply`", ); } let total_mint_value = values.iter().sum(); - self._total_supply_all.add_assign_checked( + self.total_supply_all.add_assign_checked( total_mint_value, - "should not exceed `U256::MAX` for `_total_supply_all`", + "should not exceed `U256::MAX` for `total_supply_all`", ); } @@ -254,7 +252,7 @@ impl Erc1155Supply { * values[i] <= balance_of(from, token_ids[i]) <= * total_supply(token_ids[i]) */ - self._total_supply.setter(token_id).sub_assign_unchecked(value); + self.total_supply.setter(token_id).sub_assign_unchecked(value); } let total_burn_value: U256 = values.into_iter().sum(); @@ -263,7 +261,7 @@ impl Erc1155Supply { * total_burn_value = sum_i(values[i]) <= * sum_i(total_supply(ids[i])) <= total_supply_all */ - self._total_supply_all.sub_assign_unchecked(total_burn_value); + self.total_supply_all.sub_assign_unchecked(total_burn_value); } Ok(()) } @@ -436,7 +434,7 @@ mod tests { } #[motsu::test] - #[should_panic = "should not exceed `U256::MAX` for `_total_supply`"] + #[should_panic = "should not exceed `U256::MAX` for `total_supply`"] fn mint_panics_on_total_supply_overflow(contract: Erc1155Supply) { let token_id = random_token_ids(1)[0]; let two = U256::from(2); @@ -451,7 +449,7 @@ mod tests { } #[motsu::test] - #[should_panic = "should not exceed `U256::MAX` for `_total_supply_all`"] + #[should_panic = "should not exceed `U256::MAX` for `total_supply_all`"] fn mint_panics_on_total_supply_all_overflow(contract: Erc1155Supply) { let token_ids = random_token_ids(2); contract diff --git a/contracts/src/token/erc1155/extensions/uri_storage.rs b/contracts/src/token/erc1155/extensions/uri_storage.rs index 0c62d875d..0ec04c06c 100644 --- a/contracts/src/token/erc1155/extensions/uri_storage.rs +++ b/contracts/src/token/erc1155/extensions/uri_storage.rs @@ -16,11 +16,9 @@ use super::metadata_uri::{IErc1155MetadataUri, URI}; #[storage] pub struct Erc1155UriStorage { /// Optional base URI. - #[allow(clippy::used_underscore_binding)] - pub _base_uri: StorageString, + pub(crate) base_uri: StorageString, /// Optional mapping for token URIs. - #[allow(clippy::used_underscore_binding)] - pub _token_uris: StorageMap, + pub(crate) token_uris: StorageMap, } impl Erc1155UriStorage { @@ -47,12 +45,12 @@ impl Erc1155UriStorage { token_id: U256, metadata_uri: &impl IErc1155MetadataUri, ) -> String { - let token_uri = self._token_uris.get(token_id).get_string(); + let token_uri = self.token_uris.get(token_id).get_string(); if token_uri.is_empty() { metadata_uri.uri(token_id) } else { - self._base_uri.get_string() + &token_uri + self.base_uri.get_string() + &token_uri } } @@ -75,18 +73,18 @@ impl Erc1155UriStorage { token_uri: String, metadata_uri: &impl IErc1155MetadataUri, ) { - self._token_uris.setter(token_id).set_str(token_uri); + self.token_uris.setter(token_id).set_str(token_uri); evm::log(URI { value: self.uri(token_id, metadata_uri), id: token_id }); } - /// Sets `base_uri` as the `_base_uri` for all tokens. + /// Sets `base_uri` as the `base_uri` for all tokens. /// /// # Arguments /// /// * `&mut self` - Write access to the contract's state. /// * `base_uri` - New base URI. pub fn set_base_uri(&mut self, base_uri: String) { - self._base_uri.set_str(base_uri); + self.base_uri.set_str(base_uri); } } @@ -114,7 +112,7 @@ mod tests { ) { let uri = "https://some.metadata/token/uri"; - contract.metadata_uri._uri.set_str(uri.to_owned()); + contract.metadata_uri.uri.set_str(uri.to_owned()); assert_eq!( uri, @@ -140,7 +138,7 @@ mod tests { contract .uri_storage - ._token_uris + .token_uris .setter(TOKEN_ID) .set_str(token_uri.to_owned()); @@ -157,10 +155,10 @@ mod tests { let base_uri = "https://some.base.uri"; let token_uri = "/some/token/uri"; - contract.uri_storage._base_uri.set_str(base_uri.to_owned()); + contract.uri_storage.base_uri.set_str(base_uri.to_owned()); contract .uri_storage - ._token_uris + .token_uris .setter(TOKEN_ID) .set_str(token_uri.to_owned()); @@ -177,10 +175,10 @@ mod tests { let uri = "https://some.metadata/token/uri"; let token_uri = "https://some.short/token/uri"; - contract.metadata_uri._uri.set_str(uri.to_owned()); + contract.metadata_uri.uri.set_str(uri.to_owned()); contract .uri_storage - ._token_uris + .token_uris .setter(TOKEN_ID) .set_str(token_uri.to_owned()); @@ -195,7 +193,7 @@ mod tests { let uri = "https://some.metadata/token/uri"; let token_uri = "https://some.short/token/uri".to_string(); - contract.metadata_uri._uri.set_str(uri.to_owned()); + contract.metadata_uri.uri.set_str(uri.to_owned()); contract.uri_storage.set_token_uri( TOKEN_ID, @@ -214,6 +212,6 @@ mod tests { let base_uri = "https://docs.openzeppelin.com/".to_string(); contract.set_base_uri(base_uri.clone()); - assert_eq!(base_uri, contract._base_uri.get_string()); + assert_eq!(base_uri, contract.base_uri.get_string()); } } diff --git a/contracts/src/token/erc1155/mod.rs b/contracts/src/token/erc1155/mod.rs index ae93e4764..9239ad7c6 100644 --- a/contracts/src/token/erc1155/mod.rs +++ b/contracts/src/token/erc1155/mod.rs @@ -197,11 +197,9 @@ impl MethodError for Error { #[storage] pub struct Erc1155 { /// Maps users to balances. - #[allow(clippy::used_underscore_binding)] - pub _balances: StorageMap>, + pub(crate) balances: StorageMap>, /// Maps owners to a mapping of operator approvals. - #[allow(clippy::used_underscore_binding)] - pub _operator_approvals: + pub(crate) operator_approvals: StorageMap>, } @@ -405,7 +403,7 @@ impl IErc1155 for Erc1155 { type Error = Error; fn balance_of(&self, account: Address, id: U256) -> U256 { - self._balances.get(id).get(account) + self.balances.get(id).get(account) } fn balance_of_batch( @@ -433,7 +431,7 @@ impl IErc1155 for Erc1155 { } fn is_approved_for_all(&self, account: Address, operator: Address) -> bool { - self._operator_approvals.get(account).get(operator) + self.operator_approvals.get(account).get(operator) } fn safe_transfer_from( @@ -762,7 +760,7 @@ impl Erc1155 { operator, })); } - self._operator_approvals.setter(owner).setter(operator).set(approved); + self.operator_approvals.setter(owner).setter(operator).set(approved); evm::log(ApprovalForAll { account: owner, operator, approved }); Ok(()) } @@ -1049,16 +1047,16 @@ impl Erc1155 { }, )); } - self._balances + self.balances .setter(token_id) .setter(from) .sub_assign_unchecked(value); } if !to.is_zero() { - self._balances.setter(token_id).setter(to).add_assign_checked( + self.balances.setter(token_id).setter(to).add_assign_checked( value, - "should not exceed `U256::MAX` for `_balances`", + "should not exceed `U256::MAX` for `balances`", ); } @@ -1305,7 +1303,7 @@ mod tests { #[motsu::test] fn set_approval_for_all(contract: Erc1155) { let alice = msg::sender(); - contract._operator_approvals.setter(alice).setter(BOB).set(false); + contract.operator_approvals.setter(alice).setter(BOB).set(false); contract .set_approval_for_all(BOB, true) @@ -1614,7 +1612,7 @@ mod tests { let amount_one = values[0] - uint!(1_U256); let amount_two = values[1] - uint!(1_U256); - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); contract .safe_transfer_from( @@ -1673,7 +1671,7 @@ mod tests { let invalid_sender = Address::ZERO; contract - ._operator_approvals + .operator_approvals .setter(invalid_sender) .setter(alice) .set(true); @@ -1724,7 +1722,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, BOB, 1); - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let err = contract .safe_transfer_from( @@ -1752,7 +1750,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, DAVE, 1); - contract._operator_approvals.setter(DAVE).setter(alice).set(true); + contract.operator_approvals.setter(DAVE).setter(alice).set(true); contract .safe_transfer_from( @@ -1803,7 +1801,7 @@ mod tests { let invalid_sender = Address::ZERO; contract - ._operator_approvals + .operator_approvals .setter(invalid_sender) .setter(alice) .set(true); @@ -1858,7 +1856,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, BOB, 1); - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let err = contract .safe_transfer_from( @@ -1888,7 +1886,7 @@ mod tests { let amount_one = values[0] - uint!(1_U256); let amount_two = values[1] - uint!(1_U256); - contract._operator_approvals.setter(DAVE).setter(alice).set(true); + contract.operator_approvals.setter(DAVE).setter(alice).set(true); contract .safe_batch_transfer_from( @@ -1938,7 +1936,7 @@ mod tests { let invalid_sender = Address::ZERO; contract - ._operator_approvals + .operator_approvals .setter(invalid_sender) .setter(alice) .set(true); @@ -1991,7 +1989,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, CHARLIE, 2); - contract._operator_approvals.setter(CHARLIE).setter(alice).set(true); + contract.operator_approvals.setter(CHARLIE).setter(alice).set(true); let err = contract .safe_batch_transfer_from( @@ -2019,7 +2017,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, alice, 4); - contract._operator_approvals.setter(DAVE).setter(alice).set(true); + contract.operator_approvals.setter(DAVE).setter(alice).set(true); let err = contract .safe_batch_transfer_from( @@ -2046,7 +2044,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, DAVE, 2); - contract._operator_approvals.setter(DAVE).setter(alice).set(true); + contract.operator_approvals.setter(DAVE).setter(alice).set(true); contract .safe_batch_transfer_from( @@ -2100,7 +2098,7 @@ mod tests { let invalid_sender = Address::ZERO; contract - ._operator_approvals + .operator_approvals .setter(invalid_sender) .setter(alice) .set(true); @@ -2155,7 +2153,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, CHARLIE, 2); - contract._operator_approvals.setter(CHARLIE).setter(alice).set(true); + contract.operator_approvals.setter(CHARLIE).setter(alice).set(true); let err = contract .safe_batch_transfer_from( @@ -2185,7 +2183,7 @@ mod tests { let alice = msg::sender(); let (token_ids, values) = init(contract, alice, 4); - contract._operator_approvals.setter(DAVE).setter(alice).set(true); + contract.operator_approvals.setter(DAVE).setter(alice).set(true); let err = contract .safe_batch_transfer_from( diff --git a/contracts/src/token/erc20/extensions/burnable.rs b/contracts/src/token/erc20/extensions/burnable.rs index 683531720..ba756f32f 100644 --- a/contracts/src/token/erc20/extensions/burnable.rs +++ b/contracts/src/token/erc20/extensions/burnable.rs @@ -125,7 +125,7 @@ mod tests { // Alice approves `msg::sender`. let one = uint!(1_U256); - contract._allowances.setter(alice).setter(sender).set(one); + contract.allowances.setter(alice).setter(sender).set(one); // Mint some tokens for Alice. let two = uint!(2_U256); @@ -148,7 +148,7 @@ mod tests { let zero = U256::ZERO; let one = uint!(1_U256); - contract._allowances.setter(alice).setter(msg::sender()).set(one); + contract.allowances.setter(alice).setter(msg::sender()).set(one); assert_eq!(zero, contract.balance_of(alice)); let one = uint!(1_U256); @@ -162,7 +162,7 @@ mod tests { let one = uint!(1_U256); contract - ._allowances + .allowances .setter(Address::ZERO) .setter(msg::sender()) .set(one); diff --git a/contracts/src/token/erc20/extensions/capped.rs b/contracts/src/token/erc20/extensions/capped.rs index 6b8a23c96..403fab1dc 100644 --- a/contracts/src/token/erc20/extensions/capped.rs +++ b/contracts/src/token/erc20/extensions/capped.rs @@ -21,7 +21,7 @@ mod sol { sol! { /// Indicates an error related to the operation that failed - /// because `total_supply` exceeded the `_cap`. + /// because `total_supply` exceeded the `cap`. #[derive(Debug)] #[allow(missing_docs)] error ERC20ExceededCap(uint256 increased_supply, uint256 cap); @@ -38,7 +38,7 @@ mod sol { #[derive(SolidityError, Debug)] pub enum Error { /// Indicates an error related to the operation that failed - /// because `total_supply` exceeded the `_cap`. + /// because `total_supply` exceeded the `cap`. ExceededCap(ERC20ExceededCap), /// Indicates an error related to the operation that failed /// because the supplied `cap` is not a valid cap value. @@ -49,15 +49,14 @@ pub enum Error { #[storage] pub struct Capped { /// A cap to the supply of tokens. - #[allow(clippy::used_underscore_binding)] - pub _cap: StorageU256, + pub(crate) cap: StorageU256, } #[public] impl Capped { /// Returns the cap on the token's total supply. pub fn cap(&self) -> U256 { - self._cap.get() + self.cap.get() } } @@ -70,11 +69,11 @@ mod tests { #[motsu::test] fn cap_works(contract: Capped) { let value = uint!(2024_U256); - contract._cap.set(value); + contract.cap.set(value); assert_eq!(contract.cap(), value); let value = uint!(1_U256); - contract._cap.set(value); + contract.cap.set(value); assert_eq!(contract.cap(), value); } } diff --git a/contracts/src/token/erc20/extensions/flash_mint.rs b/contracts/src/token/erc20/extensions/flash_mint.rs index 9f1629941..b53c430f4 100644 --- a/contracts/src/token/erc20/extensions/flash_mint.rs +++ b/contracts/src/token/erc20/extensions/flash_mint.rs @@ -117,9 +117,9 @@ mod borrower { #[storage] pub struct Erc20FlashMint { /// Fee applied when doing flash loans. - pub flash_fee_value: StorageU256, + pub(crate) flash_fee_value: StorageU256, /// Receiver address of the flash fee. - pub flash_fee_receiver_address: StorageAddress, + pub(crate) flash_fee_receiver_address: StorageAddress, } /// NOTE: Implementation of [`TopLevelStorage`] to be able use `&mut self` when diff --git a/contracts/src/token/erc20/extensions/metadata.rs b/contracts/src/token/erc20/extensions/metadata.rs index 3e7d0db99..d9102d8f5 100644 --- a/contracts/src/token/erc20/extensions/metadata.rs +++ b/contracts/src/token/erc20/extensions/metadata.rs @@ -21,8 +21,7 @@ use crate::utils::Metadata; #[storage] pub struct Erc20Metadata { /// Common Metadata. - #[allow(clippy::used_underscore_binding)] - pub _metadata: Metadata, + pub(crate) metadata: Metadata, } /// Interface for the optional metadata functions from the ERC-20 standard. @@ -66,11 +65,11 @@ pub trait IErc20Metadata { #[public] impl IErc20Metadata for Erc20Metadata { fn name(&self) -> String { - self._metadata.name() + self.metadata.name() } fn symbol(&self) -> String { - self._metadata.symbol() + self.metadata.symbol() } fn decimals(&self) -> u8 { diff --git a/contracts/src/token/erc20/extensions/permit.rs b/contracts/src/token/erc20/extensions/permit.rs index d46dd8f4d..d52d0f30c 100644 --- a/contracts/src/token/erc20/extensions/permit.rs +++ b/contracts/src/token/erc20/extensions/permit.rs @@ -77,11 +77,13 @@ pub enum Error { #[storage] pub struct Erc20Permit { /// ERC-20 contract. + // We leave the parent ERC-20 contract instance public, so that inheritting + // contract have access to its internal functions. pub erc20: Erc20, /// Nonces contract. - pub nonces: Nonces, + pub(crate) nonces: Nonces, /// EIP-712 contract. Must implement [`IEip712`] trait. - pub eip712: T, + pub(crate) eip712: T, } /// NOTE: Implementation of [`TopLevelStorage`] to be able use `&mut self` when diff --git a/contracts/src/token/erc20/mod.rs b/contracts/src/token/erc20/mod.rs index cf20cfa7d..bc4217a56 100644 --- a/contracts/src/token/erc20/mod.rs +++ b/contracts/src/token/erc20/mod.rs @@ -128,14 +128,12 @@ impl MethodError for Error { #[storage] pub struct Erc20 { /// Maps users to balances. - #[allow(clippy::used_underscore_binding)] - pub _balances: StorageMap, + pub(crate) balances: StorageMap, /// Maps users to a mapping of each spender's allowance. - #[allow(clippy::used_underscore_binding)] - pub _allowances: StorageMap>, + pub(crate) allowances: + StorageMap>, /// The total supply of the token. - #[allow(clippy::used_underscore_binding)] - pub _total_supply: StorageU256, + pub(crate) total_supply: StorageU256, } /// Required interface of an [`Erc20`] compliant contract. @@ -272,11 +270,11 @@ impl IErc20 for Erc20 { type Error = Error; fn total_supply(&self) -> U256 { - self._total_supply.get() + self.total_supply.get() } fn balance_of(&self, account: Address) -> U256 { - self._balances.get(account) + self.balances.get(account) } fn transfer( @@ -290,7 +288,7 @@ impl IErc20 for Erc20 { } fn allowance(&self, owner: Address, spender: Address) -> U256 { - self._allowances.get(owner).get(spender) + self.allowances.get(owner).get(spender) } fn approve( @@ -361,7 +359,7 @@ impl Erc20 { })); } - self._allowances.setter(owner).insert(spender, value); + self.allowances.setter(owner).insert(spender, value); if emit_event { evm::log(Approval { owner, spender, value }); } @@ -418,7 +416,7 @@ impl Erc20 { /// /// # Panics /// - /// If `_total_supply` exceeds `U256::MAX`. + /// If `total_supply` exceeds `U256::MAX`. /// /// # Errors /// @@ -455,7 +453,7 @@ impl Erc20 { /// /// # Panics /// - /// If `_total_supply` exceeds `U256::MAX`. It may happen during `mint` + /// If `total_supply` exceeds `U256::MAX`. It may happen during `mint` /// operation. /// /// # Errors @@ -474,13 +472,13 @@ impl Erc20 { ) -> Result<(), Error> { if from.is_zero() { // Mint operation. Overflow check required: the rest of the code - // assumes that `_total_supply` never overflows. - self._total_supply.add_assign_checked( + // assumes that `total_supply` never overflows. + self.total_supply.add_assign_checked( value, - "should not exceed `U256::MAX` for `_total_supply`", + "should not exceed `U256::MAX` for `total_supply`", ); } else { - let from_balance = self._balances.get(from); + let from_balance = self.balances.get(from); if from_balance < value { return Err(Error::InsufficientBalance( ERC20InsufficientBalance { @@ -491,20 +489,20 @@ impl Erc20 { )); } // Overflow not possible: - // `value` <= `from_balance` <= `_total_supply`. - self._balances.setter(from).set(from_balance - value); + // `value` <= `from_balance` <= `total_supply`. + self.balances.setter(from).set(from_balance - value); } if to.is_zero() { // Overflow not possible: - // `value` <= `_total_supply` or - // `value` <= `from_balance` <= `_total_supply`. - self._total_supply.sub_assign_unchecked(value); + // `value` <= `total_supply` or + // `value` <= `from_balance` <= `total_supply`. + self.total_supply.sub_assign_unchecked(value); } else { // Overflow not possible: // `balance_to` + `value` is at most `total_supply`, // which fits into a `U256`. - self._balances.setter(to).add_assign_unchecked(value); + self.balances.setter(to).add_assign_unchecked(value); } evm::log(Transfer { from, to, value }); @@ -604,7 +602,7 @@ mod tests { let owner = msg::sender(); let one = uint!(1_U256); - contract._balances.setter(owner).set(one); + contract.balances.setter(owner).set(one); let balance = contract.balance_of(owner); assert_eq!(one, balance); } @@ -628,7 +626,7 @@ mod tests { } #[motsu::test] - #[should_panic = "should not exceed `U256::MAX` for `_total_supply`"] + #[should_panic = "should not exceed `U256::MAX` for `total_supply`"] fn update_mint_errors_arithmetic_overflow(contract: Erc20) { let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d"); let one = uint!(1_U256); @@ -641,7 +639,7 @@ mod tests { ._update(Address::ZERO, alice, U256::MAX) .expect("should mint tokens"); // Mint action should NOT work: - // overflow on `_total_supply`. + // overflow on `total_supply`. let _result = contract._update(Address::ZERO, alice, one); } @@ -682,7 +680,7 @@ mod tests { } #[motsu::test] - #[should_panic = "should not exceed `U256::MAX` for `_total_supply`"] + #[should_panic = "should not exceed `U256::MAX` for `total_supply`"] fn mint_errors_arithmetic_overflow(contract: Erc20) { let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d"); let one = uint!(1_U256); @@ -694,7 +692,7 @@ mod tests { contract ._update(Address::ZERO, alice, U256::MAX) .expect("should mint tokens"); - // Mint action should NOT work -- overflow on `_total_supply`. + // Mint action should NOT work -- overflow on `total_supply`. let _result = contract._mint(alice, one); } @@ -811,7 +809,7 @@ mod tests { // Alice approves `msg::sender`. let one = uint!(1_U256); - contract._allowances.setter(alice).setter(msg::sender()).set(one); + contract.allowances.setter(alice).setter(msg::sender()).set(one); // Mint some tokens for Alice. let two = uint!(2_U256); @@ -832,7 +830,7 @@ mod tests { // Alice approves `msg::sender`. let one = uint!(1_U256); - contract._allowances.setter(alice).setter(sender).set(one); + contract.allowances.setter(alice).setter(sender).set(one); // Mint some tokens for Alice. let two = uint!(2_U256); @@ -853,7 +851,7 @@ mod tests { // Alice approves `msg::sender`. let one = uint!(1_U256); - contract._allowances.setter(alice).setter(msg::sender()).set(one); + contract.allowances.setter(alice).setter(msg::sender()).set(one); assert_eq!(U256::ZERO, contract.balance_of(alice)); let one = uint!(1_U256); @@ -866,7 +864,7 @@ mod tests { let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d"); let one = uint!(1_U256); contract - ._allowances + .allowances .setter(Address::ZERO) .setter(msg::sender()) .set(one); @@ -878,7 +876,7 @@ mod tests { fn transfer_from_errors_when_invalid_receiver(contract: Erc20) { let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d"); let one = uint!(1_U256); - contract._allowances.setter(alice).setter(msg::sender()).set(one); + contract.allowances.setter(alice).setter(msg::sender()).set(one); let result = contract.transfer_from(alice, Address::ZERO, one); assert!(matches!(result, Err(Error::InvalidReceiver(_)))); } @@ -906,7 +904,7 @@ mod tests { assert_eq!(U256::ZERO, allowance); let one = uint!(1_U256); - contract._allowances.setter(owner).setter(alice).set(one); + contract.allowances.setter(owner).setter(alice).set(one); let allowance = contract.allowance(owner, alice); assert_eq!(one, allowance); } @@ -918,7 +916,7 @@ mod tests { // `msg::sender` approves Alice. let one = uint!(1_U256); contract.approve(alice, one).unwrap(); - assert_eq!(one, contract._allowances.get(msg::sender()).get(alice)); + assert_eq!(one, contract.allowances.get(msg::sender()).get(alice)); } #[motsu::test] diff --git a/contracts/src/token/erc20/utils/safe_erc20.rs b/contracts/src/token/erc20/utils/safe_erc20.rs index 7066a1b8a..e95c9fca1 100644 --- a/contracts/src/token/erc20/utils/safe_erc20.rs +++ b/contracts/src/token/erc20/utils/safe_erc20.rs @@ -87,7 +87,7 @@ mod token { /// State of the [`SafeErc20`] Contract. #[storage] -pub struct SafeErc20 {} +pub struct SafeErc20; /// NOTE: Implementation of [`TopLevelStorage`] to be able use `&mut self` when /// calling other contracts and not `&mut (impl TopLevelStorage + diff --git a/contracts/src/token/erc721/extensions/burnable.rs b/contracts/src/token/erc721/extensions/burnable.rs index f06a90535..d3f158339 100644 --- a/contracts/src/token/erc721/extensions/burnable.rs +++ b/contracts/src/token/erc721/extensions/burnable.rs @@ -108,7 +108,7 @@ mod tests { let initial_balance = contract.balance_of(BOB).expect("should return the balance of Bob"); - contract._token_approvals.setter(TOKEN_ID).set(alice); + contract.token_approvals.setter(TOKEN_ID).set(alice); let result = contract.burn(TOKEN_ID); assert!(result.is_ok()); @@ -140,7 +140,7 @@ mod tests { contract.balance_of(BOB).expect("should return the balance of Bob"); // As we cannot change `msg::sender()`, we need to use this workaround. - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let result = contract.burn(TOKEN_ID); diff --git a/contracts/src/token/erc721/extensions/consecutive.rs b/contracts/src/token/erc721/extensions/consecutive.rs index 8211493f0..1814f6179 100644 --- a/contracts/src/token/erc721/extensions/consecutive.rs +++ b/contracts/src/token/erc721/extensions/consecutive.rs @@ -11,8 +11,8 @@ //! contract construction. This ability is regained after construction. During //! construction, only batch minting is allowed. //! -//! Fields `_first_consecutive_id` (used to offset first token id) and -//! `_max_batch_size` (used to restrict maximum batch size) can be assigned +//! Fields `first_consecutive_id` (used to offset first token id) and +//! `max_batch_size` (used to restrict maximum batch size) can be assigned //! during construction with `koba` (stylus construction tooling) within //! solidity constructor file. //! @@ -62,20 +62,16 @@ pub struct Erc721Consecutive { /// Erc721 contract storage. pub erc721: Erc721, /// Checkpoint library contract for sequential ownership. - #[allow(clippy::used_underscore_binding)] - pub _sequential_ownership: Trace, + pub(crate) sequential_ownership: Trace, /// BitMap library contract for sequential burn of tokens. - #[allow(clippy::used_underscore_binding)] - pub _sequential_burn: BitMap, + pub(crate) sequential_burn: BitMap, /// Used to offset the first token id in `next_consecutive_id` calculation. - #[allow(clippy::used_underscore_binding)] - pub _first_consecutive_id: StorageU96, + pub(crate) first_consecutive_id: StorageU96, /// Maximum size of a batch of consecutive tokens. This is designed to /// limit stress on off-chain indexing services that have to record one /// entry per token, and have protections against "unreasonably large" /// batches of tokens. - #[allow(clippy::used_underscore_binding)] - pub _max_batch_size: StorageU96, + pub(crate) max_batch_size: StorageU96, } pub use sol::*; @@ -265,11 +261,11 @@ impl Erc721Consecutive { // Otherwise, check the token was not burned, and fetch ownership from // the anchors. - if self._sequential_burn.get(token_id) { + if self.sequential_burn.get(token_id) { Address::ZERO } else { // NOTE: Bounds already checked. No need for safe cast of token_id - self._sequential_ownership.lower_lookup(U96::from(token_id)).into() + self.sequential_ownership.lower_lookup(U96::from(token_id)).into() } } @@ -335,7 +331,7 @@ impl Erc721Consecutive { // Push an ownership checkpoint & emit event. let last = next + batch_size - uint!(1_U96); - self._sequential_ownership.push(last, to.into())?; + self.sequential_ownership.push(last, to.into())?; // The invariant required by this function is preserved because the // new sequential_ownership checkpoint is attributing @@ -389,10 +385,10 @@ impl Erc721Consecutive { // and the token_id was minted in a batch && token_id < U256::from(self._next_consecutive_id()) // and the token was never marked as burnt - && !self._sequential_burn.get(token_id) + && !self.sequential_burn.get(token_id) { // record burn - self._sequential_burn.set(token_id); + self.sequential_burn.set(token_id); } Ok(previous_owner) @@ -406,7 +402,7 @@ impl Erc721Consecutive { /// /// * `&self` - Read access to the contract's state. fn _next_consecutive_id(&self) -> U96 { - match self._sequential_ownership.latest_checkpoint() { + match self.sequential_ownership.latest_checkpoint() { None => self._first_consecutive_id(), Some((latest_id, _)) => latest_id + uint!(1_U96), } @@ -415,15 +411,15 @@ impl Erc721Consecutive { /// Used to offset the first token id in /// [`Erc721Consecutive::_next_consecutive_id`]. fn _first_consecutive_id(&self) -> U96 { - self._first_consecutive_id.get() + self.first_consecutive_id.get() } /// Maximum size of consecutive token's batch. /// This is designed to limit stress on off-chain indexing services that /// have to record one entry per token, and have protections against /// "unreasonably large" batches of tokens. - fn _max_batch_size(&self) -> U96 { - self._max_batch_size.get() + pub fn _max_batch_size(&self) -> U96 { + self.max_batch_size.get() } } @@ -459,19 +455,16 @@ impl Erc721Consecutive { // event. self._approve(Address::ZERO, token_id, Address::ZERO, false)?; self.erc721 - ._balances + .balances .setter(from) .sub_assign_unchecked(uint!(1_U256)); } if !to.is_zero() { - self.erc721 - ._balances - .setter(to) - .add_assign_unchecked(uint!(1_U256)); + self.erc721.balances.setter(to).add_assign_unchecked(uint!(1_U256)); } - self.erc721._owners.setter(token_id).set(to); + self.erc721.owners.setter(token_id).set(to); evm::log(Transfer { from, to, token_id }); Ok(from) } @@ -776,7 +769,7 @@ impl Erc721Consecutive { } } - self.erc721._token_approvals.setter(token_id).set(to); + self.erc721.token_approvals.setter(token_id).set(to); Ok(()) } @@ -833,8 +826,8 @@ mod tests { receivers: Vec
, batches: Vec, ) -> Vec { - contract._first_consecutive_id.set(uint!(0_U96)); - contract._max_batch_size.set(uint!(5000_U96)); + contract.first_consecutive_id.set(uint!(0_U96)); + contract.max_batch_size.set(uint!(5000_U96)); receivers .into_iter() .zip(batches) @@ -1086,7 +1079,7 @@ mod tests { fn safe_transfers_from_approved_token(contract: Erc721Consecutive) { let alice = msg::sender(); contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); - contract.erc721._token_approvals.setter(TOKEN_ID).set(alice); + contract.erc721.token_approvals.setter(TOKEN_ID).set(alice); contract .safe_transfer_from(BOB, alice, TOKEN_ID) .expect("should transfer Bob's token to Alice"); @@ -1265,7 +1258,7 @@ mod tests { contract .approve(BOB, TOKEN_ID) .expect("should approve Bob for operations on token"); - assert_eq!(contract.erc721._token_approvals.get(TOKEN_ID), BOB); + assert_eq!(contract.erc721.token_approvals.get(TOKEN_ID), BOB); } #[motsu::test] @@ -1301,12 +1294,7 @@ mod tests { #[motsu::test] fn approval_for_all(contract: Erc721Consecutive) { let alice = msg::sender(); - contract - .erc721 - ._operator_approvals - .setter(alice) - .setter(BOB) - .set(false); + contract.erc721.operator_approvals.setter(alice).setter(BOB).set(false); contract .set_approval_for_all(BOB, true) diff --git a/contracts/src/token/erc721/extensions/enumerable.rs b/contracts/src/token/erc721/extensions/enumerable.rs index 34e3dc901..63c155b22 100644 --- a/contracts/src/token/erc721/extensions/enumerable.rs +++ b/contracts/src/token/erc721/extensions/enumerable.rs @@ -64,17 +64,13 @@ pub enum Error { #[storage] pub struct Erc721Enumerable { /// Maps owners to a mapping of indices to tokens ids. - #[allow(clippy::used_underscore_binding)] - pub _owned_tokens: StorageMap>, - /// Maps tokens ids to indices in `_owned_tokens`. - #[allow(clippy::used_underscore_binding)] - pub _owned_tokens_index: StorageMap, + pub(crate) owned_tokens: StorageMap>, + /// Maps tokens ids to indices in `owned_tokens`. + pub(crate) owned_tokens_index: StorageMap, /// Stores all tokens ids. - #[allow(clippy::used_underscore_binding)] - pub _all_tokens: StorageVec, - /// Maps indices at `_all_tokens` to tokens ids. - #[allow(clippy::used_underscore_binding)] - pub _all_tokens_index: StorageMap, + pub(crate) all_tokens: StorageVec, + /// Maps indices at `all_tokens` to tokens ids. + pub(crate) all_tokens_index: StorageMap, } /// This is the interface of the optional `Enumerable` extension @@ -138,7 +134,7 @@ impl IErc721Enumerable for Erc721Enumerable { owner: Address, index: U256, ) -> Result { - let token = self._owned_tokens.getter(owner).get(index); + let token = self.owned_tokens.getter(owner).get(index); if token.is_zero() { Err(ERC721OutOfBoundsIndex { owner, index }.into()) @@ -148,12 +144,12 @@ impl IErc721Enumerable for Erc721Enumerable { } fn total_supply(&self) -> U256 { - let tokens_length = self._all_tokens.len(); + let tokens_length = self.all_tokens.len(); U256::from(tokens_length) } fn token_by_index(&self, index: U256) -> Result { - self._all_tokens.get(index).ok_or( + self.all_tokens.get(index).ok_or( ERC721OutOfBoundsIndex { owner: Address::ZERO, index }.into(), ) } @@ -189,8 +185,8 @@ impl Erc721Enumerable { erc721: &impl IErc721, ) -> Result<(), erc721::Error> { let length = erc721.balance_of(to)? - uint!(1_U256); - self._owned_tokens.setter(to).setter(length).set(token_id); - self._owned_tokens_index.setter(token_id).set(length); + self.owned_tokens.setter(to).setter(length).set(token_id); + self.owned_tokens_index.setter(token_id).set(length); Ok(()) } @@ -205,20 +201,20 @@ impl Erc721Enumerable { pub fn _add_token_to_all_tokens_enumeration(&mut self, token_id: U256) { let index = self.total_supply(); - self._all_tokens_index.setter(token_id).set(index); - self._all_tokens.push(token_id); + self.all_tokens_index.setter(token_id).set(index); + self.all_tokens.push(token_id); } /// Function to remove a token from this extension's /// ownership-tracking data structures. /// /// Note that while the token is not assigned a new owner, - /// the `self._owned_tokens_index` mapping is NOT updated: + /// the `self.owned_tokens_index` mapping is NOT updated: /// this allows for gas optimizations e.g. /// when performing a transfer operation (avoiding double writes). /// /// This has O(1) time complexity, but alters the order - /// of the `self._owned_tokens` array. + /// of the `self.owned_tokens` array. /// /// # Arguments /// @@ -243,9 +239,9 @@ impl Erc721Enumerable { // we store the last token in the index of the token to delete, // and then delete the last slot (swap and pop). let last_token_index = erc721.balance_of(from)?; - let token_index = self._owned_tokens_index.get(token_id); + let token_index = self.owned_tokens_index.get(token_id); - let mut owned_tokens_by_owner = self._owned_tokens.setter(from); + let mut owned_tokens_by_owner = self.owned_tokens.setter(from); // When the token to delete is the last token, // the swap operation is unnecessary. @@ -255,11 +251,11 @@ impl Erc721Enumerable { // Move the last token to the slot of the to-delete token. owned_tokens_by_owner.setter(token_index).set(last_token_id); // Update the moved token's index. - self._owned_tokens_index.setter(last_token_id).set(token_index); + self.owned_tokens_index.setter(last_token_id).set(token_index); } // This also deletes the contents at the last position of the array. - self._owned_tokens_index.delete(token_id); + self.owned_tokens_index.delete(token_id); owned_tokens_by_owner.delete(last_token_index); Ok(()) @@ -269,7 +265,7 @@ impl Erc721Enumerable { /// token tracking data structures. /// /// This has O(1) time complexity, - /// but alters the order of the `self._all_tokens` array. + /// but alters the order of the `self.all_tokens` array. /// /// # Arguments /// @@ -286,8 +282,8 @@ impl Erc721Enumerable { // To prevent a gap in the tokens array, // we store the last token in the index of the token to delete, // and then delete the last slot (swap and pop). - let last_token_index = U256::from(self._all_tokens.len() - 1); - let token_index = self._all_tokens_index.get(token_id); + let last_token_index = U256::from(self.all_tokens.len() - 1); + let token_index = self.all_tokens_index.get(token_id); // When the token to delete is the last token, // the swap operation is unnecessary. @@ -297,22 +293,22 @@ impl Erc721Enumerable { // to avoid the gas cost of adding an 'if' statement // (like in `self._remove_token_from_owner_enumeration`). let last_token_id = self - ._all_tokens + .all_tokens .get(last_token_index) .expect("token at given index must exist"); // Move the last token to the slot of the to-delete token. - self._all_tokens + self.all_tokens .setter(token_index) .expect("slot at given `token_index` must exist") .set(last_token_id); // Update the moved token's index. - self._all_tokens_index.setter(last_token_id).set(token_index); + self.all_tokens_index.setter(last_token_id).set(token_index); // This also deletes the contents at the last position of the array. - self._all_tokens_index.delete(token_id); - self._all_tokens.pop(); + self.all_tokens_index.delete(token_id); + self.all_tokens.pop(); } /// See [`crate::token::erc721::Erc721::_increase_balance`]. diff --git a/contracts/src/token/erc721/extensions/metadata.rs b/contracts/src/token/erc721/extensions/metadata.rs index cc417c674..866cfae1a 100644 --- a/contracts/src/token/erc721/extensions/metadata.rs +++ b/contracts/src/token/erc721/extensions/metadata.rs @@ -20,11 +20,9 @@ use crate::{ #[storage] pub struct Erc721Metadata { /// Common Metadata. - #[allow(clippy::used_underscore_binding)] - pub _metadata: Metadata, + pub(crate) metadata: Metadata, /// Base URI for tokens. - #[allow(clippy::used_underscore_binding)] - pub _base_uri: StorageString, + pub(crate) base_uri: StorageString, } /// Interface for the optional metadata functions from the ERC-721 standard. @@ -51,11 +49,11 @@ pub trait IErc721Metadata { #[public] impl IErc721Metadata for Erc721Metadata { fn name(&self) -> String { - self._metadata.name() + self.metadata.name() } fn symbol(&self) -> String { - self._metadata.symbol() + self.metadata.symbol() } } @@ -79,7 +77,7 @@ impl Erc721Metadata { /// /// * `&self` - Read access to the contract's state. pub fn base_uri(&self) -> String { - self._base_uri.get_string() + self.base_uri.get_string() } /// Returns the Uniform Resource Identifier (URI) for `token_id` token. diff --git a/contracts/src/token/erc721/extensions/uri_storage.rs b/contracts/src/token/erc721/extensions/uri_storage.rs index 96fdc33a4..bb8273f34 100644 --- a/contracts/src/token/erc721/extensions/uri_storage.rs +++ b/contracts/src/token/erc721/extensions/uri_storage.rs @@ -37,8 +37,7 @@ mod sol { #[storage] pub struct Erc721UriStorage { /// Optional mapping for token URIs. - #[allow(clippy::used_underscore_binding)] - pub _token_uris: StorageMap, + pub token_uris: StorageMap, } impl Erc721UriStorage { @@ -53,7 +52,7 @@ impl Erc721UriStorage { /// # Events /// Emits a [`MetadataUpdate`] event. pub fn _set_token_uri(&mut self, token_id: U256, token_uri: String) { - self._token_uris.setter(token_id).set_str(token_uri); + self.token_uris.setter(token_id).set_str(token_uri); evm::log(MetadataUpdate { token_id }); } @@ -93,7 +92,7 @@ impl Erc721UriStorage { ) -> Result { let _owner = erc721.owner_of(token_id)?; - let token_uri = self._token_uris.getter(token_id).get_string(); + let token_uri = self.token_uris.getter(token_id).get_string(); let base = metadata.base_uri(); // If there is no base URI, return the token URI. @@ -144,7 +143,7 @@ mod tests { let token_uri = String::from("https://docs.openzeppelin.com/contracts/5.x/api/token/erc721#Erc721URIStorage"); contract .uri_storage - ._token_uris + .token_uris .setter(TOKEN_ID) .set_str(token_uri.clone()); @@ -169,7 +168,7 @@ mod tests { let initial_token_uri = String::from("https://docs.openzeppelin.com/contracts/5.x/api/token/erc721#Erc721URIStorage"); contract .uri_storage - ._token_uris + .token_uris .setter(TOKEN_ID) .set_str(initial_token_uri); diff --git a/contracts/src/token/erc721/mod.rs b/contracts/src/token/erc721/mod.rs index 21a9b0c4f..7802e5d3b 100644 --- a/contracts/src/token/erc721/mod.rs +++ b/contracts/src/token/erc721/mod.rs @@ -178,17 +178,13 @@ impl MethodError for Error { #[storage] pub struct Erc721 { /// Maps tokens to owners. - #[allow(clippy::used_underscore_binding)] - pub _owners: StorageMap, + pub(crate) owners: StorageMap, /// Maps users to balances. - #[allow(clippy::used_underscore_binding)] - pub _balances: StorageMap, + pub(crate) balances: StorageMap, /// Maps tokens to approvals. - #[allow(clippy::used_underscore_binding)] - pub _token_approvals: StorageMap, + pub(crate) token_approvals: StorageMap, /// Maps owners to a mapping of operator approvals. - #[allow(clippy::used_underscore_binding)] - pub _operator_approvals: + pub(crate) operator_approvals: StorageMap>, } @@ -472,7 +468,7 @@ impl IErc721 for Erc721 { if owner.is_zero() { return Err(ERC721InvalidOwner { owner: Address::ZERO }.into()); } - Ok(self._balances.get(owner)) + Ok(self.balances.get(owner)) } fn owner_of(&self, token_id: U256) -> Result { @@ -545,7 +541,7 @@ impl IErc721 for Erc721 { } fn is_approved_for_all(&self, owner: Address, operator: Address) -> bool { - self._operator_approvals.get(owner).get(operator) + self.operator_approvals.get(owner).get(operator) } } @@ -576,7 +572,7 @@ impl Erc721 { /// * `token_id` - Token id as a number. #[must_use] pub fn _owner_of(&self, token_id: U256) -> Address { - self._owners.get(token_id) + self.owners.get(token_id) } /// Returns the approved address for `token_id`. @@ -588,7 +584,7 @@ impl Erc721 { /// * `token_id` - Token id as a number. #[must_use] pub fn _get_approved(&self, token_id: U256) -> Address { - self._token_approvals.get(token_id) + self.token_approvals.get(token_id) } /// Returns whether `spender` is allowed to manage `owner`'s tokens, or @@ -673,7 +669,7 @@ impl Erc721 { /// * `account` - Account to increase balance. /// * `value` - The number of tokens to increase balance. pub fn _increase_balance(&mut self, account: Address, value: U128) { - self._balances.setter(account).add_assign_unchecked(U256::from(value)); + self.balances.setter(account).add_assign_unchecked(U256::from(value)); } /// Transfers `token_id` from its current owner to `to`, or alternatively @@ -723,14 +719,14 @@ impl Erc721 { // Clear approval. No need to re-authorize or emit the `Approval` // event. self._approve(Address::ZERO, token_id, Address::ZERO, false)?; - self._balances.setter(from).sub_assign_unchecked(uint!(1_U256)); + self.balances.setter(from).sub_assign_unchecked(uint!(1_U256)); } if !to.is_zero() { - self._balances.setter(to).add_assign_unchecked(uint!(1_U256)); + self.balances.setter(to).add_assign_unchecked(uint!(1_U256)); } - self._owners.setter(token_id).set(to); + self.owners.setter(token_id).set(to); evm::log(Transfer { from, to, token_id }); Ok(from) } @@ -1014,7 +1010,7 @@ impl Erc721 { } } - self._token_approvals.setter(token_id).set(to); + self.token_approvals.setter(token_id).set(to); Ok(()) } @@ -1049,7 +1045,7 @@ impl Erc721 { return Err(ERC721InvalidOperator { operator }.into()); } - self._operator_approvals.setter(owner).setter(operator).set(approved); + self.operator_approvals.setter(owner).setter(operator).set(approved); evm::log(ApprovalForAll { owner, operator, approved }); Ok(()) } @@ -1326,7 +1322,7 @@ mod tests { fn transfers_from_approved_token(contract: Erc721) { let alice = msg::sender(); contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); - contract._token_approvals.setter(TOKEN_ID).set(alice); + contract.token_approvals.setter(TOKEN_ID).set(alice); contract .transfer_from(BOB, alice, TOKEN_ID) .expect("should transfer Bob's token to Alice"); @@ -1342,7 +1338,7 @@ mod tests { contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); // As we cannot change `msg::sender`, we need to use this workaround. - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let approved_for_all = contract.is_approved_for_all(BOB, alice); assert!(approved_for_all); @@ -1462,7 +1458,7 @@ mod tests { fn safe_transfers_from_approved_token(contract: Erc721) { let alice = msg::sender(); contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); - contract._token_approvals.setter(TOKEN_ID).set(alice); + contract.token_approvals.setter(TOKEN_ID).set(alice); contract .safe_transfer_from(BOB, alice, TOKEN_ID) .expect("should transfer Bob's token to Alice"); @@ -1478,7 +1474,7 @@ mod tests { contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); // As we cannot change `msg::sender()`, we need to use this workaround. - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let approved_for_all = contract.is_approved_for_all(BOB, alice); assert!(approved_for_all); @@ -1603,7 +1599,7 @@ mod tests { fn safe_transfers_from_with_data_approved_token(contract: Erc721) { let alice = msg::sender(); contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); - contract._token_approvals.setter(TOKEN_ID).set(alice); + contract.token_approvals.setter(TOKEN_ID).set(alice); contract .safe_transfer_from_with_data( BOB, @@ -1624,7 +1620,7 @@ mod tests { contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); // As we cannot change `msg::sender()`, we need to use this workaround. - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let approved_for_all = contract.is_approved_for_all(BOB, alice); assert!(approved_for_all); @@ -1760,7 +1756,7 @@ mod tests { contract .approve(BOB, TOKEN_ID) .expect("should approve Bob for operations on token"); - assert_eq!(contract._token_approvals.get(TOKEN_ID), BOB); + assert_eq!(contract.token_approvals.get(TOKEN_ID), BOB); } #[motsu::test] @@ -1796,7 +1792,7 @@ mod tests { #[motsu::test] fn approval_for_all(contract: Erc721) { let alice = msg::sender(); - contract._operator_approvals.setter(alice).setter(BOB).set(false); + contract.operator_approvals.setter(alice).setter(BOB).set(false); contract .set_approval_for_all(BOB, true) @@ -2096,7 +2092,7 @@ mod tests { fn transfers_approved_token(contract: Erc721) { let alice = msg::sender(); contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); - contract._token_approvals.setter(TOKEN_ID).set(alice); + contract.token_approvals.setter(TOKEN_ID).set(alice); contract ._transfer(BOB, alice, TOKEN_ID) .expect("should transfer Bob's token to Alice"); @@ -2112,7 +2108,7 @@ mod tests { contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); // As we cannot change `msg::sender`, we need to use this workaround. - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let approved_for_all = contract.is_approved_for_all(BOB, alice); assert!(approved_for_all); @@ -2211,7 +2207,7 @@ mod tests { fn safe_transfers_internal_approved_token(contract: Erc721) { let alice = msg::sender(); contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); - contract._token_approvals.setter(TOKEN_ID).set(alice); + contract.token_approvals.setter(TOKEN_ID).set(alice); contract ._safe_transfer(BOB, alice, TOKEN_ID, &vec![0, 1, 2, 3].into()) .expect("should transfer Bob's token to Alice"); @@ -2227,7 +2223,7 @@ mod tests { contract._mint(BOB, TOKEN_ID).expect("should mint token to Bob"); // As we cannot change `msg::sender()`, we need to use this workaround. - contract._operator_approvals.setter(BOB).setter(alice).set(true); + contract.operator_approvals.setter(BOB).setter(alice).set(true); let approved_for_all = contract.is_approved_for_all(BOB, alice); assert!(approved_for_all); @@ -2320,7 +2316,7 @@ mod tests { contract ._approve(BOB, TOKEN_ID, alice, false) .expect("should approve Bob for operations on token"); - assert_eq!(contract._token_approvals.get(TOKEN_ID), BOB); + assert_eq!(contract.token_approvals.get(TOKEN_ID), BOB); } #[motsu::test] @@ -2357,7 +2353,7 @@ mod tests { #[motsu::test] fn approval_for_all_internal(contract: Erc721) { let alice = msg::sender(); - contract._operator_approvals.setter(alice).setter(BOB).set(false); + contract.operator_approvals.setter(alice).setter(BOB).set(false); contract ._set_approval_for_all(alice, BOB, true) diff --git a/contracts/src/utils/cryptography/eip712.rs b/contracts/src/utils/cryptography/eip712.rs index f69d63809..18a24dd23 100644 --- a/contracts/src/utils/cryptography/eip712.rs +++ b/contracts/src/utils/cryptography/eip712.rs @@ -147,7 +147,7 @@ mod tests { address!("000000000000000000000000000000000000dEaD"); #[derive(Default)] - struct TestEIP712 {} + struct TestEIP712; impl IEip712 for TestEIP712 { const NAME: &'static str = "A Name"; diff --git a/contracts/src/utils/metadata.rs b/contracts/src/utils/metadata.rs index 0365bf5a7..188458618 100644 --- a/contracts/src/utils/metadata.rs +++ b/contracts/src/utils/metadata.rs @@ -9,11 +9,9 @@ use stylus_sdk::{ #[storage] pub struct Metadata { /// Token name. - #[allow(clippy::used_underscore_binding)] - pub _name: StorageString, + pub(crate) name: StorageString, /// Token symbol. - #[allow(clippy::used_underscore_binding)] - pub _symbol: StorageString, + pub(crate) symbol: StorageString, } #[public] @@ -24,7 +22,7 @@ impl Metadata { /// /// * `&self` - Read access to the contract's state. pub fn name(&self) -> String { - self._name.get_string() + self.name.get_string() } /// Returns the symbol of the token, usually a shorter version of the name. @@ -33,6 +31,6 @@ impl Metadata { /// /// * `&self` - Read access to the contract's state. pub fn symbol(&self) -> String { - self._symbol.get_string() + self.symbol.get_string() } } diff --git a/contracts/src/utils/nonces.rs b/contracts/src/utils/nonces.rs index 4a588af5f..769fe8d7e 100644 --- a/contracts/src/utils/nonces.rs +++ b/contracts/src/utils/nonces.rs @@ -39,8 +39,7 @@ pub enum Error { #[storage] pub struct Nonces { /// Mapping from address to its nonce. - #[allow(clippy::used_underscore_binding)] - pub _nonces: StorageMap, + pub(crate) nonces: StorageMap, } #[public] @@ -53,7 +52,7 @@ impl Nonces { /// * `owner` - The address for which to return the nonce. #[must_use] pub fn nonces(&self, owner: Address) -> U256 { - self._nonces.get(owner) + self.nonces.get(owner) } } @@ -69,9 +68,9 @@ impl Nonces { /// /// If the nonce for the given `owner` exceeds `U256::MAX`. pub fn use_nonce(&mut self, owner: Address) -> U256 { - let nonce = self._nonces.get(owner); + let nonce = self.nonces.get(owner); - self._nonces + self.nonces .setter(owner) .add_assign_checked(ONE, "nonce should not exceed `U256::MAX`"); diff --git a/contracts/src/utils/pausable.rs b/contracts/src/utils/pausable.rs index 60e369628..a4d8d0f30 100644 --- a/contracts/src/utils/pausable.rs +++ b/contracts/src/utils/pausable.rs @@ -68,8 +68,7 @@ pub enum Error { #[storage] pub struct Pausable { /// Indicates whether the contract is `Paused`. - #[allow(clippy::used_underscore_binding)] - pub _paused: StorageBool, + pub(crate) paused: StorageBool, } #[public] @@ -80,7 +79,7 @@ impl Pausable { /// /// * `&self` - Read access to the contract's state. fn paused(&self) -> bool { - self._paused.get() + self.paused.get() } } @@ -97,7 +96,7 @@ impl Pausable { /// [`Error::EnforcedPause`] is returned. pub fn pause(&mut self) -> Result<(), Error> { self.when_not_paused()?; - self._paused.set(true); + self.paused.set(true); evm::log(Paused { account: msg::sender() }); Ok(()) } @@ -114,7 +113,7 @@ impl Pausable { /// [`Error::ExpectedPause`] is returned. pub fn unpause(&mut self) -> Result<(), Error> { self.when_paused()?; - self._paused.set(false); + self.paused.set(false); evm::log(Unpaused { account: msg::sender() }); Ok(()) } @@ -131,7 +130,7 @@ impl Pausable { /// If the contract is in the `Paused` state, then the error /// [`Error::EnforcedPause`] is returned. pub fn when_not_paused(&self) -> Result<(), Error> { - if self._paused.get() { + if self.paused.get() { return Err(Error::EnforcedPause(EnforcedPause {})); } Ok(()) @@ -149,7 +148,7 @@ impl Pausable { /// If the contract is in `Unpaused` state, then the error /// [`Error::ExpectedPause`] is returned. pub fn when_paused(&self) -> Result<(), Error> { - if !self._paused.get() { + if !self.paused.get() { return Err(Error::ExpectedPause(ExpectedPause {})); } Ok(()) @@ -162,15 +161,15 @@ mod tests { #[motsu::test] fn paused_works(contract: Pausable) { - contract._paused.set(false); + contract.paused.set(false); assert!(!contract.paused()); - contract._paused.set(true); + contract.paused.set(true); assert!(contract.paused()); } #[motsu::test] fn when_not_paused_works(contract: Pausable) { - contract._paused.set(false); + contract.paused.set(false); assert!(!contract.paused()); let result = contract.when_not_paused(); @@ -179,7 +178,7 @@ mod tests { #[motsu::test] fn when_not_paused_errors_when_paused(contract: Pausable) { - contract._paused.set(true); + contract.paused.set(true); assert!(contract.paused()); let result = contract.when_not_paused(); @@ -188,7 +187,7 @@ mod tests { #[motsu::test] fn when_paused_works(contract: Pausable) { - contract._paused.set(true); + contract.paused.set(true); assert!(contract.paused()); let result = contract.when_paused(); @@ -197,7 +196,7 @@ mod tests { #[motsu::test] fn when_paused_errors_when_not_paused(contract: Pausable) { - contract._paused.set(false); + contract.paused.set(false); assert!(!contract.paused()); let result = contract.when_paused(); @@ -206,7 +205,7 @@ mod tests { #[motsu::test] fn pause_works(contract: Pausable) { - contract._paused.set(false); + contract.paused.set(false); assert!(!contract.paused()); // Pause the contract @@ -217,7 +216,7 @@ mod tests { #[motsu::test] fn pause_errors_when_already_paused(contract: Pausable) { - contract._paused.set(true); + contract.paused.set(true); assert!(contract.paused()); let result = contract.pause(); @@ -227,7 +226,7 @@ mod tests { #[motsu::test] fn unpause_works(contract: Pausable) { - contract._paused.set(true); + contract.paused.set(true); assert!(contract.paused()); // Unpause the paused contract @@ -238,7 +237,7 @@ mod tests { #[motsu::test] fn unpause_errors_when_already_unpaused(contract: Pausable) { - contract._paused.set(false); + contract.paused.set(false); assert!(!contract.paused()); // Unpause the unpaused contract diff --git a/contracts/src/utils/structs/bitmap.rs b/contracts/src/utils/structs/bitmap.rs index 40f808a18..8d8a03f1e 100644 --- a/contracts/src/utils/structs/bitmap.rs +++ b/contracts/src/utils/structs/bitmap.rs @@ -26,8 +26,7 @@ const HEX_FF: U256 = uint!(0xff_U256); #[storage] pub struct BitMap { /// Inner laying mapping. - #[allow(clippy::used_underscore_binding)] - pub _data: StorageMap, + pub(crate) data: StorageMap, } impl BitMap { @@ -40,7 +39,7 @@ impl BitMap { pub fn get(&self, index: U256) -> bool { let bucket = Self::get_bucket(index); let mask = Self::get_mask(index); - let value = self._data.get(bucket); + let value = self.data.get(bucket); (value & mask) != U256::ZERO } @@ -66,7 +65,7 @@ impl BitMap { pub fn set(&mut self, index: U256) { let bucket = Self::get_bucket(index); let mask = Self::get_mask(index); - let mut value = self._data.setter(bucket); + let mut value = self.data.setter(bucket); let prev = value.get(); value.set(prev | mask); } @@ -79,7 +78,7 @@ impl BitMap { pub fn unset(&mut self, index: U256) { let bucket = Self::get_bucket(index); let mask = Self::get_mask(index); - let mut value = self._data.setter(bucket); + let mut value = self.data.setter(bucket); let prev = value.get(); value.set(prev & !mask); } diff --git a/contracts/src/utils/structs/checkpoints/mod.rs b/contracts/src/utils/structs/checkpoints/mod.rs index 81d522f58..3fc038042 100644 --- a/contracts/src/utils/structs/checkpoints/mod.rs +++ b/contracts/src/utils/structs/checkpoints/mod.rs @@ -51,19 +51,16 @@ impl MethodError for Error { #[storage] pub struct Trace { /// Stores checkpoints in a dynamic array sorted by key. - #[allow(clippy::used_underscore_binding)] - pub _checkpoints: StorageVec>, + pub(crate) checkpoints: StorageVec>, } /// State of a single checkpoint. #[storage] pub struct Checkpoint { /// The key of the checkpoint. Used as a sorting key. - #[allow(clippy::used_underscore_binding)] - pub _key: S::KeyStorage, + pub(crate) key: S::KeyStorage, /// The value corresponding to the key. - #[allow(clippy::used_underscore_binding)] - pub _value: S::ValueStorage, + pub(crate) value: S::ValueStorage, } impl Trace { @@ -107,7 +104,7 @@ impl Trace { if pos == len { S::Value::ZERO } else { - self._index(pos)._value.get() + self._index(pos).value.get() } } @@ -125,7 +122,7 @@ impl Trace { if pos == U256::ZERO { S::Value::ZERO } else { - self._index(pos - uint!(1_U256))._value.get() + self._index(pos - uint!(1_U256)).value.get() } } @@ -147,7 +144,7 @@ impl Trace { if len > uint!(5_U256) { let mid = len - len.sqrt(); - if key < self._index(mid)._key.get() { + if key < self._index(mid).key.get() { high = mid; } else { low = mid + uint!(1_U256); @@ -159,7 +156,7 @@ impl Trace { if pos == U256::ZERO { S::Value::ZERO } else { - self._index(pos - uint!(1_U256))._value.get() + self._index(pos - uint!(1_U256)).value.get() } } @@ -174,7 +171,7 @@ impl Trace { if pos == U256::ZERO { S::Value::ZERO } else { - self._index(pos - uint!(1_U256))._value.get() + self._index(pos - uint!(1_U256)).value.get() } } @@ -191,7 +188,7 @@ impl Trace { None } else { let checkpoint = self._index(pos - uint!(1_U256)); - Some((checkpoint._key.get(), checkpoint._value.get())) + Some((checkpoint.key.get(), checkpoint.value.get())) } } @@ -201,7 +198,7 @@ impl Trace { /// /// * `&self` - Read access to the checkpoint's state. pub fn length(&self) -> U256 { - U256::from(self._checkpoints.len()) + U256::from(self.checkpoints.len()) } /// Returns checkpoint at given position. @@ -215,10 +212,10 @@ impl Trace { /// * `&self` - Read access to the checkpoint's state. /// * `pos` - Index of the checkpoint. pub fn at(&self, pos: U32) -> (S::Key, S::Value) { - let guard = self._checkpoints.get(pos).unwrap_or_else(|| { + let guard = self.checkpoints.get(pos).unwrap_or_else(|| { panic!("should get checkpoint at index `{pos}`") }); - (guard._key.get(), guard._value.get()) + (guard.key.get(), guard.value.get()) } /// Pushes a (`key`, `value`) pair into an ordered list of checkpoints, @@ -244,8 +241,8 @@ impl Trace { let pos = self.length(); if pos > U256::ZERO { let last = self._index(pos - uint!(1_U256)); - let last_key = last._key.get(); - let last_value = last._value.get(); + let last_key = last.key.get(); + let last_value = last.value.get(); // Checkpoint keys must be non-decreasing. if last_key > key { @@ -254,7 +251,7 @@ impl Trace { // Update or push new checkpoint if last_key == key { - self._index_mut(pos - uint!(1_U256))._value.set(value); + self._index_mut(pos - uint!(1_U256)).value.set(value); } else { self._unchecked_push(key, value); } @@ -287,7 +284,7 @@ impl Trace { ) -> U256 { while low < high { let mid = low.average(high); - if self._index(mid)._key.get() > key { + if self._index(mid).key.get() > key { high = mid; } else { low = mid + uint!(1_U256); @@ -318,7 +315,7 @@ impl Trace { ) -> U256 { while low < high { let mid = low.average(high); - if self._index(mid)._key.get() < key { + if self._index(mid).key.get() < key { low = mid + uint!(1_U256); } else { high = mid; @@ -339,7 +336,7 @@ impl Trace { /// * `&self` - Read access to the checkpoint's state. /// * `pos` - Index of the checkpoint. fn _index(&self, pos: U256) -> StorageGuard> { - self._checkpoints + self.checkpoints .get(pos) .unwrap_or_else(|| panic!("should get checkpoint at index `{pos}`")) } @@ -356,7 +353,7 @@ impl Trace { /// * `&mut self` - Write access to the checkpoint's state. /// * `pos` - Index of the checkpoint. fn _index_mut(&mut self, pos: U256) -> StorageGuardMut> { - self._checkpoints + self.checkpoints .setter(pos) .unwrap_or_else(|| panic!("should get checkpoint at index `{pos}`")) } @@ -369,9 +366,9 @@ impl Trace { /// * `key` - Checkpoint key to insert. /// * `value` - Checkpoint value corresponding to insertion `key`. fn _unchecked_push(&mut self, key: S::Key, value: S::Value) { - let mut new_checkpoint = self._checkpoints.grow(); - new_checkpoint._key.set(key); - new_checkpoint._value.set(value); + let mut new_checkpoint = self.checkpoints.grow(); + new_checkpoint.key.set(key); + new_checkpoint.value.set(value); } } diff --git a/examples/ownable-two-step/src/lib.rs b/examples/ownable-two-step/src/lib.rs index 79f4a9a77..03ffcc13c 100644 --- a/examples/ownable-two-step/src/lib.rs +++ b/examples/ownable-two-step/src/lib.rs @@ -27,7 +27,7 @@ impl Ownable2StepExample { to: Address, value: U256, ) -> Result<(), Vec> { - self.ownable._ownable.only_owner()?; + self.ownable.ownable.only_owner()?; self.erc20.transfer(to, value)?; Ok(()) }