From 024e22533d8df9b03e971bb725fef035d044d812 Mon Sep 17 00:00:00 2001 From: 0xNeshi Date: Fri, 6 Dec 2024 12:16:42 +0100 Subject: [PATCH 1/5] ref: change all receiver-related magic bytes to function_selector calcs --- contracts/src/token/erc721/mod.rs | 11 ++++++----- examples/erc1155/tests/erc1155.rs | 10 +++++----- examples/erc1155/tests/mock/receiver.rs | 22 +++++++++++++++++++--- examples/erc721/tests/mock/receiver.rs | 16 +++++++++++----- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/contracts/src/token/erc721/mod.rs b/contracts/src/token/erc721/mod.rs index 5f6fb886d..aec4346f6 100644 --- a/contracts/src/token/erc721/mod.rs +++ b/contracts/src/token/erc721/mod.rs @@ -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::*, }; @@ -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. /// @@ -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(()); } @@ -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()); } diff --git a/examples/erc1155/tests/erc1155.rs b/examples/erc1155/tests/erc1155.rs index 612e23b04..f985cfc6e 100644 --- a/examples/erc1155/tests/erc1155.rs +++ b/examples/erc1155/tests/erc1155.rs @@ -2,7 +2,7 @@ use abi::Erc1155; use alloy::{ - primitives::{fixed_bytes, uint, Address, U256}, + primitives::{uint, Address, U256}, sol, }; use e2e::{receipt, send, watch, Account, EventExt, ReceiptExt, Revert}; @@ -201,7 +201,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 } = @@ -452,7 +452,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 } = @@ -903,7 +903,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 } = @@ -1340,7 +1340,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 diff --git a/examples/erc1155/tests/mock/receiver.rs b/examples/erc1155/tests/mock/receiver.rs index da021c4cb..6959954ab 100644 --- a/examples/erc1155/tests/mock/receiver.rs +++ b/examples/erc1155/tests/mock/receiver.rs @@ -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, + Vec, + Bytes +)); sol! { #[allow(missing_docs)] diff --git a/examples/erc721/tests/mock/receiver.rs b/examples/erc721/tests/mock/receiver.rs index f759227e4..2b49b8d1c 100644 --- a/examples/erc721/tests/mock/receiver.rs +++ b/examples/erc721/tests/mock/receiver.rs @@ -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)] @@ -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
{ - 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()) } From 41424e265fdb856ba142dc128604f618e2ecd040 Mon Sep 17 00:00:00 2001 From: 0xNeshi Date: Tue, 10 Dec 2024 10:43:40 +0100 Subject: [PATCH 2/5] test: add supports_interface motsu test --- .../src/token/erc721/extensions/metadata.rs | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/contracts/src/token/erc721/extensions/metadata.rs b/contracts/src/token/erc721/extensions/metadata.rs index c1e95e078..08f258e9e 100644 --- a/contracts/src/token/erc721/extensions/metadata.rs +++ b/contracts/src/token/erc721/extensions/metadata.rs @@ -21,6 +21,9 @@ sol_storage! { } } +// 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 + /// Interface for the optional metadata functions from the ERC-721 standard. #[interface_id] pub trait IErc721Metadata { @@ -53,11 +56,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) + (::INTERFACE_ID ^ TOKEN_URI_SELECTOR) + == u32::from_be_bytes(*interface_id) } } @@ -116,14 +123,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 = ::INTERFACE_ID; - // let expected = 0x5b5e139f; - // assert_eq!(actual, expected); - // } + use super::{Erc721Metadata, IErc165, IErc721Metadata}; + + #[motsu::test] + fn interface_id() { + let actual = ::INTERFACE_ID; + let expected = 0x93254542; + assert_eq!(actual, expected); + } + + #[motsu::test] + fn supports_interface() { + assert!(::supports_interface( + 0x5b5e139f.into() + )); + } } From 833342affdda6683cc84459e4e81f8dc63a89d17 Mon Sep 17 00:00:00 2001 From: 0xNeshi Date: Tue, 10 Dec 2024 10:44:03 +0100 Subject: [PATCH 3/5] ref(test): rename all tests support_interface->supports_interface --- examples/erc1155-metadata-uri/tests/erc1155-metadata-uri.rs | 2 +- examples/erc1155/tests/erc1155.rs | 2 +- examples/erc20/tests/erc20.rs | 2 +- examples/erc721-metadata/tests/erc721.rs | 2 +- examples/erc721/tests/erc721.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/erc1155-metadata-uri/tests/erc1155-metadata-uri.rs b/examples/erc1155-metadata-uri/tests/erc1155-metadata-uri.rs index e9f853005..d57642c2c 100644 --- a/examples/erc1155-metadata-uri/tests/erc1155-metadata-uri.rs +++ b/examples/erc1155-metadata-uri/tests/erc1155-metadata-uri.rs @@ -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)) diff --git a/examples/erc1155/tests/erc1155.rs b/examples/erc1155/tests/erc1155.rs index 84d68e6c7..fd8926e69 100644 --- a/examples/erc1155/tests/erc1155.rs +++ b/examples/erc1155/tests/erc1155.rs @@ -1863,7 +1863,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; diff --git a/examples/erc20/tests/erc20.rs b/examples/erc20/tests/erc20.rs index b16ffa9ca..f2ea9cb19 100644 --- a/examples/erc20/tests/erc20.rs +++ b/examples/erc20/tests/erc20.rs @@ -1322,7 +1322,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::() diff --git a/examples/erc721-metadata/tests/erc721.rs b/examples/erc721-metadata/tests/erc721.rs index f8edde696..de604757d 100644 --- a/examples/erc721-metadata/tests/erc721.rs +++ b/examples/erc721-metadata/tests/erc721.rs @@ -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( diff --git a/examples/erc721/tests/erc721.rs b/examples/erc721/tests/erc721.rs index de471d101..fa3af9adb 100644 --- a/examples/erc721/tests/erc721.rs +++ b/examples/erc721/tests/erc721.rs @@ -2079,7 +2079,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; From c8d14af52e9f1d95a00153fb9c440dc94360e1f3 Mon Sep 17 00:00:00 2001 From: 0xNeshi Date: Tue, 10 Dec 2024 10:48:27 +0100 Subject: [PATCH 4/5] chore: update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bea83cf7..24e99ea71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,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 `MethodError` for `safe_erc20::Error`. #402 - Use `function_selector!` to calculate transfer type selector in `Erc1155`. #417 - Update internal functions of `Erc721` and `Erc721Consecutive` accept reference to `Bytes`. #437 From d3d42a01bd07390ee6f93ca6f43b63dfb98a8043 Mon Sep 17 00:00:00 2001 From: Nenad Date: Tue, 10 Dec 2024 11:58:56 +0100 Subject: [PATCH 5/5] docs: remove TODO comment from erc721 metadata --- contracts/src/token/erc721/extensions/metadata.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/src/token/erc721/extensions/metadata.rs b/contracts/src/token/erc721/extensions/metadata.rs index 08f258e9e..ff19adbd9 100644 --- a/contracts/src/token/erc721/extensions/metadata.rs +++ b/contracts/src/token/erc721/extensions/metadata.rs @@ -21,9 +21,6 @@ sol_storage! { } } -// 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 - /// Interface for the optional metadata functions from the ERC-721 standard. #[interface_id] pub trait IErc721Metadata {