Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

precompile erc 721 only owner_of #104

Merged
merged 61 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
8a0bc02
create mock and testing
asiniscalchi Aug 1, 2023
0fe9a63
first test
asiniscalchi Aug 1, 2023
7ebabef
testing ethereum reserved addresses
asiniscalchi Aug 1, 2023
e964668
refactoring
asiniscalchi Aug 1, 2023
aba874e
refactoring
asiniscalchi Aug 1, 2023
80dd172
refactoring tests
asiniscalchi Aug 1, 2023
9604f6c
fmt
asiniscalchi Aug 1, 2023
390359b
erc721 starting point
asiniscalchi Aug 3, 2023
64c449a
Merge branch 'dev' into feature/precompile_erc_721
asiniscalchi Aug 3, 2023
5dafd9f
fmt
asiniscalchi Aug 3, 2023
e92faae
Merge branch 'dev' into feature/precompile_erc_721
asiniscalchi Aug 3, 2023
21d6842
Merge branch 'feature/solidity_create_collection_return_an_address' i…
asiniscalchi Aug 4, 2023
e06adce
compiling
asiniscalchi Aug 4, 2023
3869690
check selectors
asiniscalchi Aug 4, 2023
d514f88
create trait Erc721
asiniscalchi Aug 4, 2023
2efff92
Erc721Precompile
asiniscalchi Aug 4, 2023
0357938
set mocks
asiniscalchi Aug 4, 2023
cec3cbf
first implermentation
asiniscalchi Aug 4, 2023
b0c10ce
first integration of erc721 in the runtime
asiniscalchi Aug 4, 2023
1420c21
fmt
asiniscalchi Aug 4, 2023
d0c4ce7
check on the collection id
asiniscalchi Aug 4, 2023
aa84dde
test on extract owner form asset_id
asiniscalchi Aug 4, 2023
7e49dae
Merge branch 'dev' into feature/precompile_erc_721
asiniscalchi Aug 5, 2023
0ec72a7
fix tests
asiniscalchi Aug 5, 2023
9be1a5b
test for erc721 trait
asiniscalchi Aug 5, 2023
2427e0d
Merge branch 'feature/precompile_erc_721' into feature/testing_precom…
asiniscalchi Aug 5, 2023
4625aad
fix compilation
asiniscalchi Aug 5, 2023
d858bf0
refactoring
asiniscalchi Aug 5, 2023
6c65ddd
returning address
asiniscalchi Aug 6, 2023
1c9e12a
return value is correctly encoded as a n Address
asiniscalchi Aug 7, 2023
c7ec2ee
Merge branch 'bug/create_collection_logs' into feature/precompile_erc…
asiniscalchi Aug 7, 2023
c39efbd
Merge branch 'feature/testing_precompiles' into feature/precompile_er…
asiniscalchi Aug 7, 2023
9dc0c14
is_erc721_contract is not member of PrecompoileSet
asiniscalchi Aug 7, 2023
b9bba2d
test on precompiled contracts
asiniscalchi Aug 7, 2023
a39432c
erc721 returns hardcoded address
asiniscalchi Aug 7, 2023
c4cd42f
refactoring
asiniscalchi Aug 7, 2023
89af0a0
testing return of asset ownership
asiniscalchi Aug 7, 2023
c6c7810
fix errors
asiniscalchi Aug 7, 2023
5cfb2b5
remove unused use
asiniscalchi Aug 7, 2023
fa612ab
Merge branch 'dev' into feature/precompile_erc_721
asiniscalchi Aug 7, 2023
0568c47
refactoring
asiniscalchi Aug 7, 2023
b31f1db
rewriting testing mod
asiniscalchi Aug 7, 2023
9a4f100
refactoring
asiniscalchi Aug 7, 2023
09738af
moving check for cllection address in pallet
asiniscalchi Aug 8, 2023
daf2910
collection address prefix changed
asiniscalchi Aug 8, 2023
bd47de2
test passing
asiniscalchi Aug 8, 2023
907ca7e
test passing
asiniscalchi Aug 8, 2023
10557aa
refactoring tests
asiniscalchi Aug 8, 2023
6d38abe
do not panic the runtime
asiniscalchi Aug 8, 2023
daef051
fmt
asiniscalchi Aug 8, 2023
f71eef7
solidity tokenId -> _tokenId
asiniscalchi Aug 8, 2023
15123db
erc721 functions are views
asiniscalchi Aug 8, 2023
4b0ec80
update docs
asiniscalchi Aug 8, 2023
df59324
sp-core in std , removed duplicate function
asiniscalchi Aug 8, 2023
2fbd144
documentaetion
asiniscalchi Aug 8, 2023
96d7374
using compilator type ifer
asiniscalchi Aug 8, 2023
5bf67b0
compiler infer the size of the array
asiniscalchi Aug 8, 2023
2bf2171
added to std feature
asiniscalchi Aug 8, 2023
9e1bf8d
rename variable
asiniscalchi Aug 8, 2023
88e1314
trait documentation
asiniscalchi Aug 8, 2023
2aa2681
fmt
asiniscalchi Aug 8, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
389 changes: 200 additions & 189 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ pallet-bridge-relayers = { git = "https://github.com/freeverseio/parity-bridges-
# LAOS pallets
pallet-living-assets-ownership = { path = "./pallets/living-assets-ownership", default-features = false }
pallet-evm-living-assets-ownership = { path = "./precompile/living-assets", default-features = false }
pallet-evm-erc721 = { path = "./precompile/erc721", default-features = false }

# Utils
precompile-utils = { path = "./precompile/utils", default-features = false }
Expand Down
6 changes: 5 additions & 1 deletion pallets/living-assets-ownership/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ edition = "2021"
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
hex-literal = { workspace = true, optional = true}
parity-scale-codec = { workspace = true, features = ["derive"] }
scale-info = { workspace = true, features = ["derive"] }

Expand All @@ -20,9 +21,11 @@ frame-benchmarking = { workspace = true, optional = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
sp-arithmetic = { workspace = true }
sp-core = { workspace = true }
This conversation was marked as resolved.
Show resolved Hide resolved

[dev-dependencies]
serde = { workspace = true }
hex = { version = "0.4.3" }

# Substrate
sp-core = { workspace = true }
Expand All @@ -42,7 +45,8 @@ std = [
"frame-benchmarking/std",
"frame-support/std",
"frame-system/std",
"sp-arithmetic/std"
"sp-arithmetic/std",
"sp-core/std",
]
try-runtime = [
"frame-system/try-runtime",
Expand Down
20 changes: 20 additions & 0 deletions pallets/living-assets-ownership/src/functions.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Contains helper and utility functions of the pallet
use super::*;
use frame_support::sp_runtime::traits::One;
use sp_core::{H160, U256};

impl<T: Config> Pallet<T> {
/// See [Self::create_collection]
Expand All @@ -23,3 +24,22 @@ impl<T: Config> Pallet<T> {
Ok(collection_id)
}
}

pub fn convert_asset_id_to_owner(value: U256) -> H160 {
asiniscalchi marked this conversation as resolved.
Show resolved Hide resolved
let mut bytes = [0u8; 20];
let value_bytes: [u8; 32] = value.into();
bytes.copy_from_slice(&value_bytes[value_bytes.len() - 20..]);
H160::from(bytes)
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn check_convert_asset_id_to_owner() {
let value = U256::from(5);
let expected_address = H160::from_low_u64_be(5);
assert_eq!(convert_asset_id_to_owner(value), expected_address);
}
}
87 changes: 87 additions & 0 deletions pallets/living-assets-ownership/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@
/// Learn more about FRAME and the core library of Substrate FRAME pallets:
/// <https://docs.substrate.io/reference/frame-pallets/>
pub use pallet::*;
use sp_core::H160;

mod functions;
pub mod traits;

#[frame_support::pallet]
pub mod pallet {
use crate::functions::convert_asset_id_to_owner;

use super::*;
use frame_support::pallet_prelude::{OptionQuery, ValueQuery, *};
use frame_system::pallet_prelude::*;
use sp_core::{H160, U256};

/// Collection id type
/// TODO: use 256 bits
Expand Down Expand Up @@ -83,6 +87,89 @@ pub mod pallet {
Self::do_create_collection(owner)
}
}

impl<T: Config> traits::Erc721 for Pallet<T> {
fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result<H160, &'static str> {
match OwnerOfCollection::<T>::get(collection_id) {
Some(_) => Ok(convert_asset_id_to_owner(asset_id)),
None => Err("Collection does not exist"),
}
}
}
}

/// `ASSET_PRECOMPILE_ADDRESS_PREFIX` is a predefined prefix used to identify collection addresses.
///
/// All addresses that start with this prefix are considered as collection addresses.
/// Since `CollectionId` is represented as a `u64`, it leaves these bits free to be
/// utilized for such a prefix.
///
/// Usage of this prefix provides a consistent and recognizable pattern for distinguishing
/// collection addresses from other types of addresses in the system.
pub const ASSET_PRECOMPILE_ADDRESS_PREFIX: &[u8] = &[0xff; 12];

/// Enum representing possible errors related to collections.
#[derive(Debug, PartialEq)]
pub enum CollectionError {
/// Error indicating that the provided address does not have the correct prefix.
InvalidPrefix,
}

/// Converts a `CollectionId` into an `H160` address format.
///
/// This function takes the given `CollectionId`, which is assumed to be a `u64`,
/// and maps it into an `H160` address, prepending it with the `ASSET_PRECOMPILE_ADDRESS_PREFIX`.
///
/// # Arguments
///
/// * `collection_id`: The ID of the collection to be converted.
///
/// # Returns
///
/// * An `H160` representation of the collection ID.
pub fn collection_id_to_address(collection_id: CollectionId) -> H160 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really unidiomatic for Rust and Substrate, generally. Either create a trait for all these functions, sth like LivingAssetsUtils and implement this trait for Pallet or to some dummy struct and use them to call these functions or just move these functions under impl<T: Config> Pallet<T> {} as static functions.

they are also missing comments

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'll be adressed : #116

let mut bytes = [0u8; 20];
bytes[12..20].copy_from_slice(&collection_id.to_be_bytes());
for (i, byte) in ASSET_PRECOMPILE_ADDRESS_PREFIX.iter().enumerate() {
bytes[i] = *byte;
}
H160(bytes)
}

/// Converts an `H160` address into a `CollectionId` format.
///
/// This function takes the given `H160` address, checks for the correct prefix, and extracts
/// the `CollectionId` from it. If the prefix is incorrect, it returns a `CollectionError::InvalidPrefix` error.
///
/// # Arguments
///
/// * `address`: The `H160` address to be converted.
///
/// # Returns
///
/// * A `Result` which is either the `CollectionId` or an error indicating the address is invalid.
pub fn address_to_collection_id(address: H160) -> Result<CollectionId, CollectionError> {
if &address.0[0..12] != ASSET_PRECOMPILE_ADDRESS_PREFIX {
return Err(CollectionError::InvalidPrefix);
}
let id_bytes: [u8; 8] = address.0[12..].try_into().unwrap();
Ok(CollectionId::from_be_bytes(id_bytes))
}

/// Checks if a given `H160` address is a collection address.
///
/// This function examines the prefix of the given `H160` address to determine if it is a
/// collection address, based on the `ASSET_PRECOMPILE_ADDRESS_PREFIX`.
///
/// # Arguments
///
/// * `address`: The `H160` address to be checked.
///
/// # Returns
///
/// * A boolean indicating if the address is a collection address.
pub fn is_collection_address(address: H160) -> bool {
&address.to_fixed_bytes()[0..12] == ASSET_PRECOMPILE_ADDRESS_PREFIX
}

#[cfg(test)]
Expand Down
68 changes: 67 additions & 1 deletion pallets/living-assets-ownership/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
use crate::{mock::*, traits::CollectionManager, Event};
use core::str::FromStr;

use crate::{
address_to_collection_id, collection_id_to_address, is_collection_address,
mock::*,
traits::{CollectionManager, Erc721},
CollectionError, Event,
};
use frame_support::assert_ok;
use sp_core::H160;

type AccountId = <Test as frame_system::Config>::AccountId;

Expand Down Expand Up @@ -107,3 +115,61 @@ fn living_assets_ownership_trait_id_of_new_collection_should_be_consecutive() {
);
});
}

#[test]
fn erc721_owner_of_asset_of_unexistent_collection() {
new_test_ext().execute_with(|| {
assert_eq!(
<LivingAssetsModule as Erc721>::owner_of(0, 2.into()),
Err("Collection does not exist")
);
});
}

#[test]
fn erc721_owner_of_asset_of_collection() {
new_test_ext().execute_with(|| {
let collection_id =
<LivingAssetsModule as CollectionManager<AccountId>>::create_collection(ALICE).unwrap();
assert_eq!(
<LivingAssetsModule as Erc721>::owner_of(collection_id, 2.into()).unwrap(),
H160::from_low_u64_be(0x0000000000000002)
);
});
}

#[test]
fn test_collection_id_to_address() {
let collection_id = 5;
let expected_address = H160::from_str("ffffffffffffffffffffffff0000000000000005").unwrap();
assert_eq!(collection_id_to_address(collection_id), expected_address);
}

#[test]
fn invalid_collection_address_should_error() {
let address = H160::from_str("8000000000000000000000000000000000000005").unwrap();
let error = address_to_collection_id(address).unwrap_err();
assert_eq!(error, CollectionError::InvalidPrefix);
}

#[test]
fn valid_collection_address_should_return_collection_id() {
let address = H160::from_str("ffffffffffffffffffffffff0000000000000005").unwrap();
let collection_id = address_to_collection_id(address).unwrap();
assert_eq!(collection_id, 5);
}

#[test]
fn test_is_collection_address_valid() {
let collection_id = 1234567890;
let address = collection_id_to_address(collection_id);

assert!(is_collection_address(address));
}

#[test]
fn test_is_collection_address_invalid() {
let invalid_address = H160([0u8; 20]);

assert!(!is_collection_address(invalid_address));
}
6 changes: 6 additions & 0 deletions pallets/living-assets-ownership/src/traits.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use sp_core::{H160, U256};

use crate::CollectionId;

/// The `CollectionManager` trait provides an interface for managing collections in a
Expand Down Expand Up @@ -25,3 +27,7 @@ pub trait CollectionManager<AccountId> {
/// Create collection
fn create_collection(owner: AccountId) -> Result<CollectionId, &'static str>;
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is missing documentation. you can use the comments to leave TODO mark for complete implementation of ERC721 as well.

pub trait Erc721 {
fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result<H160, &'static str>;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also forgot to mention that it's not a good practice to use str for errors, try to use explicit Error type which you can pass to the trait or type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes totally agree.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be addressed here : #116

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result<H160, &'static str>;
type Error;
fn owner_of(collection_id: CollectionId, asset_id: U256) -> Result<H160, &'static str>;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sth like this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
55 changes: 55 additions & 0 deletions precompile/erc721/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
[package]
name = "pallet-evm-erc721"
version = "0.0.1"
edition = "2021"

[dependencies]
parity-scale-codec = { workspace = true, features = [
"derive",
] }
scale-info = { workspace = true, features = [
"derive",
] }

# Frontier
fp-evm = { workspace = true }
pallet-evm = { workspace = true }

# Substrate
frame-support = { workspace = true }
sp-arithmetic = { workspace = true }
sp-core = { workspace = true }
sp-runtime = { workspace = true }
sp-std = { workspace = true }

# Local pallet
pallet-living-assets-ownership = { workspace = true }

# Utils
precompile-utils = { workspace = true }
precompile-utils-macro = { workspace = true }

num_enum = { workspace = true }

[dev-dependencies]
evm = { workspace = true }
hex = { version = "0.4.3" }
precompile-utils = { workspace = true, features = ["testing"]}

[features]
default = ["std"]
std = [
# Frontier
"fp-evm/std",
"pallet-evm/std",
"sp-core/std",
"sp-runtime/std",
"sp-std/std",
"pallet-living-assets-ownership/std",
"num_enum/std",
"frame-support/std",
"sp-arithmetic/std",
"precompile-utils/std",
"parity-scale-codec/std",
"scale-info/std",
]
12 changes: 12 additions & 0 deletions precompile/erc721/contracts/IERC721.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// SPDX-License-Identifier: MIT
// derived from OpenZeppelin Contracts (last updated v4.9.0) (token/ERC721/IERC721.sol)
pragma solidity >=0.8.3;

interface IERC721 {
/**
* @dev See {IERC721Metadata-tokenURI}.
*/
function tokenURI(uint256 _tokenId) external view returns (string memory);

function ownerOf(uint256 _tokenId) external view returns (address);
}
Loading