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

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

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)
+        (<Self as IErc721Metadata>::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 = <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()
+        ));
+    }
 }

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());
         }
 

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))

examples/erc1155/tests/erc1155.rs

@@ -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};
 
@@ -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 } =
@@ -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 } =
@@ -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 } =
@@ -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
@@ -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;

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<U256>,
+    Vec<U256>,
+    Bytes
+));
 
 sol! {
     #[allow(missing_docs)]

examples/erc20/tests/erc20.rs

@@ -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>()

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(

examples/erc721/tests/erc721.rs

@@ -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;

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<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())
 }