Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ref: Update All fn Selector "Magic Values" Into Explicit fn Selector Calculation #442

Merged
merged 8 commits into from
Dec 10, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Update "magic values" to explicit calculations in `Erc721Metadata::supports_interface`, and `Erc721::_check_on_erc721_received`. #442
- Implement `AddAssignUnchecked` and `SubAssignUnchecked` for `StorageUint`. #418
- Implement `MethodError` for `safe_erc20::Error`. #402
- Use `function_selector!` to calculate transfer type selector in `Erc1155`. #417
Expand Down
31 changes: 20 additions & 11 deletions contracts/src/token/erc721/extensions/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,15 @@ impl IErc721Metadata for Erc721Metadata {
}
}

const TOKEN_URI_SELECTOR: u32 =
u32::from_be_bytes(stylus_sdk::function_selector!("tokenURI", U256));

impl IErc165 for Erc721Metadata {
fn supports_interface(interface_id: FixedBytes<4>) -> bool {
// NOTE: interface id is calculated using additional selector
// [`Erc721Metadata::token_uri`]
0x_5b5e139f == u32::from_be_bytes(*interface_id)
(<Self as IErc721Metadata>::INTERFACE_ID ^ TOKEN_URI_SELECTOR)
== u32::from_be_bytes(*interface_id)
}
}

Expand Down Expand Up @@ -116,14 +120,19 @@ impl Erc721Metadata {

#[cfg(all(test, feature = "std"))]
mod tests {
// use crate::token::erc721::extensions::{Erc721Metadata, IErc721Metadata};

// TODO: IErc721Metadata should be refactored to have same api as solidity
// has: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4764ea50750d8bda9096e833706beba86918b163/contracts/token/ERC721/extensions/IERC721Metadata.sol#L12
// [motsu::test]
// fn interface_id() {
// let actual = <Erc721Metadata as IErc721Metadata>::INTERFACE_ID;
// let expected = 0x5b5e139f;
// assert_eq!(actual, expected);
// }
use super::{Erc721Metadata, IErc165, IErc721Metadata};

#[motsu::test]
fn interface_id() {
let actual = <Erc721Metadata as IErc721Metadata>::INTERFACE_ID;
let expected = 0x93254542;
assert_eq!(actual, expected);
}

#[motsu::test]
fn supports_interface() {
assert!(<Erc721Metadata as IErc165>::supports_interface(
0x5b5e139f.into()
));
}
}
11 changes: 6 additions & 5 deletions contracts/src/token/erc721/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//! Implementation of the [`Erc721`] token standard.
use alloc::vec;

use alloy_primitives::{fixed_bytes, uint, Address, FixedBytes, U128, U256};
use alloy_primitives::{uint, Address, FixedBytes, U128, U256};
use openzeppelin_stylus_proc::interface_id;
use stylus_sdk::{
abi::Bytes,
alloy_sol_types::sol,
call::{self, Call, MethodError},
evm, msg,
evm, function_selector, msg,
prelude::*,
};

Expand Down Expand Up @@ -565,6 +565,9 @@ impl IErc165 for Erc721 {
}

impl Erc721 {
const RECEIVER_FN_SELECTOR: [u8; 4] =
function_selector!("onERC721Received", Address, Address, U256, Bytes,);

/// Returns the owner of the `token_id`. Does NOT revert if the token
/// doesn't exist.
///
Expand Down Expand Up @@ -1115,8 +1118,6 @@ impl Erc721 {
token_id: U256,
data: &Bytes,
) -> Result<(), Error> {
const RECEIVER_FN_SELECTOR: FixedBytes<4> = fixed_bytes!("150b7a02");

if !to.has_code() {
return Ok(());
}
Expand Down Expand Up @@ -1146,7 +1147,7 @@ impl Erc721 {
};

// Token rejected.
if id != RECEIVER_FN_SELECTOR {
if id != Self::RECEIVER_FN_SELECTOR {
return Err(ERC721InvalidReceiver { receiver: to }.into());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ async fn uri_ignores_metadata_uri_when_token_uri_is_set(
// ============================================================================

#[e2e::test]
async fn support_interface(alice: Account) -> eyre::Result<()> {
async fn supports_interface(alice: Account) -> eyre::Result<()> {
let contract_addr = alice
.as_deployer()
.with_constructor(ctr(URI))
Expand Down
12 changes: 6 additions & 6 deletions examples/erc1155/tests/erc1155.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![cfg(feature = "e2e")]

use abi::Erc1155;
use alloy::primitives::{fixed_bytes, uint, Address, U256};
use alloy::primitives::{uint, Address, U256};
use e2e::{receipt, send, watch, Account, EventExt, ReceiptExt, Revert};
use mock::{receiver, receiver::ERC1155ReceiverMock};

Expand Down Expand Up @@ -153,7 +153,7 @@ async fn mints_to_receiver_contract(alice: Account) -> eyre::Result<()> {
from: Address::ZERO,
id: token_id,
value,
data: fixed_bytes!("").into(),
data: vec![].into(),
}));

let Erc1155::balanceOfReturn { balance: receiver_balance } =
Expand Down Expand Up @@ -367,7 +367,7 @@ async fn mint_batch_transfer_to_receiver_contract(
from: Address::ZERO,
ids: token_ids.clone(),
values: values.clone(),
data: fixed_bytes!("").into(),
data: vec![].into(),
}));

let Erc1155::balanceOfBatchReturn { balances: receiver_balances } =
Expand Down Expand Up @@ -763,7 +763,7 @@ async fn safe_transfer_to_receiver_contract(
from: alice_addr,
id: token_id,
value,
data: fixed_bytes!("").into(),
data: vec![].into(),
}));

let Erc1155::balanceOfReturn { balance: alice_balance } =
Expand Down Expand Up @@ -1155,7 +1155,7 @@ async fn safe_batch_transfer_to_receiver_contract(
from: alice_addr,
ids: token_ids.clone(),
values: values.clone(),
data: fixed_bytes!("").into(),
data: vec![].into(),
}));

let Erc1155::balanceOfBatchReturn { balances: alice_balances } = contract
Expand Down Expand Up @@ -1860,7 +1860,7 @@ async fn error_when_insufficient_balance_burn_batch(
// ============================================================================

#[e2e::test]
async fn support_interface(alice: Account) -> eyre::Result<()> {
async fn supports_interface(alice: Account) -> eyre::Result<()> {
let contract_addr = alice.as_deployer().deploy().await?.address()?;
let contract = Erc1155::new(contract_addr, &alice.wallet);
let invalid_interface_id: u32 = 0xffffffff;
Expand Down
22 changes: 19 additions & 3 deletions examples/erc1155/tests/mock/receiver.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
#![allow(dead_code)]
#![cfg(feature = "e2e")]
use alloy::{
primitives::{fixed_bytes, Address, FixedBytes},
primitives::{Address, FixedBytes, U256},
sol,
};
use e2e::Wallet;
use stylus_sdk::{abi::Bytes, function_selector};

const REC_RETVAL: FixedBytes<4> = fixed_bytes!("f23a6e61");
const BAT_RETVAL: FixedBytes<4> = fixed_bytes!("bc197c81");
const REC_RETVAL: FixedBytes<4> = FixedBytes(function_selector!(
"onERC1155Received",
Address,
Address,
U256,
U256,
Bytes
));

const BAT_RETVAL: FixedBytes<4> = FixedBytes(function_selector!(
"onERC1155BatchReceived",
Address,
Address,
Vec<U256>,
Vec<U256>,
Bytes
));

sol! {
#[allow(missing_docs)]
Expand Down
2 changes: 1 addition & 1 deletion examples/erc20/tests/erc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ async fn error_when_transfer_from(alice: Account, bob: Account) -> Result<()> {
// ============================================================================

#[e2e::test]
async fn support_interface(alice: Account) -> Result<()> {
async fn supports_interface(alice: Account) -> Result<()> {
let contract_addr = alice
.as_deployer()
.with_default_constructor::<constructorCall>()
Expand Down
2 changes: 1 addition & 1 deletion examples/erc721-metadata/tests/erc721.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ async fn return_token_uri_after_burn_and_remint(
// ============================================================================

#[e2e::test]
async fn support_interface(alice: Account) -> eyre::Result<()> {
async fn supports_interface(alice: Account) -> eyre::Result<()> {
let contract_addr = alice
.as_deployer()
.with_constructor(ctr(
Expand Down
2 changes: 1 addition & 1 deletion examples/erc721/tests/erc721.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2113,7 +2113,7 @@ async fn token_by_index_after_burn_and_some_mints(
// ============================================================================

#[e2e::test]
async fn support_interface(alice: Account) -> eyre::Result<()> {
async fn supports_interface(alice: Account) -> eyre::Result<()> {
let contract_addr = alice.as_deployer().deploy().await?.address()?;
let contract = Erc721::new(contract_addr, &alice.wallet);
let invalid_interface_id: u32 = 0x_ffffffff;
Expand Down
16 changes: 11 additions & 5 deletions examples/erc721/tests/mock/receiver.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
#![allow(dead_code)]
#![cfg(feature = "e2e")]
use alloy::{
primitives::{fixed_bytes, Address},
primitives::{Address, FixedBytes, U256},
sol,
};
use e2e::Wallet;
use stylus_sdk::{abi::Bytes, function_selector};

sol! {
#[allow(missing_docs)]
Expand Down Expand Up @@ -55,13 +56,18 @@ sol! {
}
}

const RET_VAL: FixedBytes<4> = FixedBytes(function_selector!(
"onERC721Received",
Address,
Address,
U256,
Bytes,
));

pub async fn deploy(
wallet: &Wallet,
error: ERC721ReceiverMock::RevertType,
) -> eyre::Result<Address> {
let retval = fixed_bytes!("150b7a02");

// Deploy the contract.
let contract = ERC721ReceiverMock::deploy(wallet, retval, error).await?;
let contract = ERC721ReceiverMock::deploy(wallet, RET_VAL, error).await?;
Ok(*contract.address())
}
Loading