Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions examples/Odra.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,6 @@ fqn = "factory::token::FToken"

[[contracts]]
fqn = "factory::token::FactoryProxy"

[[contracts]]
fqn = "contracts::cep96_cep95::Cep96Cep95"
178 changes: 178 additions & 0 deletions examples/src/contracts/cep96_cep95.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
//! An example of a CEP-95 token with CEP-96 contract metadata.
use odra::casper_types::bytesrepr::Bytes;
use odra::casper_types::U256;
use odra::prelude::*;
use odra_modules::cep95::{CEP95Interface, Cep95};
use odra_modules::cep96::{Cep96, Cep96ContractMetadata};

/// CEP-95 token with CEP-96 contract metadata.
#[odra::module]
pub struct Cep96Cep95 {
token: SubModule<Cep95>,
metadata: SubModule<Cep96>
}

#[odra::module]
impl Cep96Cep95 {
/// Initializes the contract with CEP-95 token params and CEP-96 metadata.
pub fn init(
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

The example contract Cep96Cep95 initializes the Cep95 submodule with self.token.init(name, symbol). The related context for Cep95 (templates and other examples) shows that Cep95::init typically sets the name and symbol via self.name.set() and self.symbol.set(). The provided tests pass, indicating the integration works. However, this example does not demonstrate or test error scenarios, such as initializing metadata twice (which would be silently ignored) or unauthorized mint/burn attempts. The tests are positive cases only. Adding negative test cases would improve robustness and serve as better documentation for the module's behavior.

&mut self,
name: String,
symbol: String,
contract_name: Option<String>,
contract_description: Option<String>,
contract_icon_uri: Option<String>,
contract_project_uri: Option<String>
) {
self.token.init(name, symbol);
self.metadata.init(
contract_name,
contract_description,
contract_icon_uri,
contract_project_uri
);
}

delegate! {
to self.token {
fn name(&self) -> String;
fn symbol(&self) -> String;
fn balance_of(&self, owner: Address) -> U256;
fn owner_of(&self, token_id: U256) -> Option<Address>;
fn safe_transfer_from(&mut self, from: Address, to: Address, token_id: U256, data: Option<Bytes>);
fn transfer_from(&mut self, from: Address, to: Address, token_id: U256);
fn approve(&mut self, spender: Address, token_id: U256);
fn revoke_approval(&mut self, token_id: U256);
fn approved_for(&self, token_id: U256) -> Option<Address>;
fn approve_for_all(&mut self, operator: Address);
fn revoke_approval_for_all(&mut self, operator: Address);
fn is_approved_for_all(&self, owner: Address, operator: Address) -> bool;
fn token_metadata(&self, token_id: U256) -> Vec<(String, String)>;
}
}

delegate! {
to self.metadata {
fn contract_name(&self) -> Option<String>;
fn contract_description(&self) -> Option<String>;
fn contract_icon_uri(&self) -> Option<String>;
fn contract_project_uri(&self) -> Option<String>;
}
}

/// Mints a new token with the given ID and metadata to the specified address.
pub fn mint(&mut self, to: Address, token_id: U256, metadata: Vec<(String, String)>) {
self.token.raw_mint(to, token_id, metadata);
}

/// Burns the token with the given ID.
pub fn burn(&mut self, token_id: U256) {
let owner = self.token.owner_of(token_id);
let caller = self.env().caller();
if Some(caller) == owner {
self.token.raw_burn(token_id);
}
}
Comment on lines +69 to +75
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

The burn function silently does nothing if the caller is not the token owner. This is inconsistent with the pattern established in the related owned_cep95.rs contract, where the function also does nothing on a non-owner call. However, this is a flawed pattern that can lead to user confusion and failed transactions without clear error messages. Burning is a critical, irreversible operation; the contract should either revert with a clear error (e.g., "Caller is not the token owner") or implement proper authorization logic (e.g., allowing an approved operator or a contract owner to burn). The silent failure masks intent and makes debugging difficult.

Code Suggestion:

pub fn burn(&mut self, token_id: U256) {
    let owner = self.token.owner_of(token_id)
        .unwrap_or_else(|| self.env().revert(Error::TokenNonExistent)); // Assuming an Error enum exists.
    let caller = self.env().caller();
    if caller != owner {
        // Also consider checking `is_approved_for_all` or `approved_for` if operators should be allowed.
        self.env().revert(Error::Unauthorized);
    }
    self.token.raw_burn(token_id);
}

Evidence: path:examples/src/contracts/owned_cep95.rs, method:burn

}

#[cfg(test)]
mod test {
use super::Cep96Cep95;
use crate::contracts::cep96_cep95::{Cep96Cep95HostRef, Cep96Cep95InitArgs};
use odra::{host::Deployer, prelude::*};
use odra_test;

fn deploy_contract() -> Cep96Cep95HostRef {
let env = odra_test::env();
Cep96Cep95::deploy(
&env,
Cep96Cep95InitArgs {
name: "TestToken".to_string(),
symbol: "TST".to_string(),
contract_name: Some("My NFT Collection".to_string()),
contract_description: Some("A test NFT collection".to_string()),
contract_icon_uri: Some("https://example.com/icon.png".to_string()),
contract_project_uri: Some("https://example.com".to_string())
}
)
}

#[test]
fn test_cep96_metadata() {
let contract = deploy_contract();

// Verify all CEP-96 metadata fields
assert_eq!(
contract.contract_name(),
Some("My NFT Collection".to_string())
);
assert_eq!(
contract.contract_description(),
Some("A test NFT collection".to_string())
);
assert_eq!(
contract.contract_icon_uri(),
Some("https://example.com/icon.png".to_string())
);
assert_eq!(
contract.contract_project_uri(),
Some("https://example.com".to_string())
);
}

#[test]
fn test_cep96_partial_metadata() {
let env = odra_test::env();
let contract = Cep96Cep95::deploy(
&env,
Cep96Cep95InitArgs {
name: "TestToken".to_string(),
symbol: "TST".to_string(),
contract_name: Some("Partial Collection".to_string()),
contract_description: None,
contract_icon_uri: None,
contract_project_uri: Some("https://example.com".to_string())
}
);

// Only set fields should have values
assert_eq!(
contract.contract_name(),
Some("Partial Collection".to_string())
);
assert_eq!(contract.contract_description(), None);
assert_eq!(contract.contract_icon_uri(), None);
assert_eq!(
contract.contract_project_uri(),
Some("https://example.com".to_string())
);
}

#[test]
fn test_cep95_with_cep96() {
let env = odra_test::env();
let mut contract = deploy_contract();

// CEP-95 functionality should work
assert_eq!(contract.name(), "TestToken");
assert_eq!(contract.symbol(), "TST");

// Mint a token
let owner = env.caller();
let token_id = 1.into();
contract.mint(
owner,
token_id,
vec![("key".to_string(), "value".to_string())]
);

assert_eq!(contract.owner_of(token_id), Some(owner));
assert_eq!(contract.balance_of(owner), 1.into());

// CEP-96 metadata should still be accessible
assert_eq!(
contract.contract_name(),
Some("My NFT Collection".to_string())
);
}
}
1 change: 1 addition & 0 deletions examples/src/contracts/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Module containing examples of contracts written in Odra.
pub mod balance_checker;
pub mod cep96_cep95;
pub mod owned_cep95;
pub mod owned_token;
pub mod tlw;
Expand Down
72 changes: 72 additions & 0 deletions modules/src/cep96.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
#![allow(unused_variables, missing_docs)]

mod storage;

use odra::prelude::*;

use crate::cep96::storage::{
Cep96DescriptionStorage, Cep96IconUriStorage, Cep96NameStorage, Cep96ProjectUriStorage
};
pub trait Cep96ContractMetadata {
/// Contract's human-readable name.
fn contract_name(&self) -> Option<String>;

/// Brief description of the contract.
fn contract_description(&self) -> Option<String>;

/// URI pointing to the contract's icon image.
fn contract_icon_uri(&self) -> Option<String>;

Comment on lines +10 to +19
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

The module starts with #![allow(missing_docs)], and the public trait Cep96ContractMetadata lacks comprehensive documentation. While the field names are descriptive, there is no documentation explaining the standard (CEP-96) this implements, the expected format of URIs (e.g., HTTPS, IPFS), maximum length constraints, or the mutability semantics (immutable after first set, as per the storage implementation). This lack of documentation increases the learning curve for developers and the risk of improper integration. Public traits that define a standard's interface should be thoroughly documented.

Code Suggestion:

//! Implementation of the Casper CEP-96 Standard: Contract Metadata.
//! Provides a standard interface for retrieving metadata about a smart contract (name, description, icon, project link).

/// Trait defining the CEP-96 standard interface for contract metadata.
/// All fields are optional as per the standard.
pub trait Cep96ContractMetadata {
    /// Returns the human-readable name of the contract (e.g., "My NFT Collection").
    /// Returns `None` if not set.
    fn contract_name(&self) -> Option<String>;

    /// Returns a brief description of the contract's purpose.
    /// Returns `None` if not set.
    fn contract_description(&self) -> Option<String>;

    /// Returns a URI (e.g., HTTPS, IPFS) pointing to an icon image representing the contract.
    /// The image should be square and at least 512x512 pixels.
    /// Returns `None` if not set.
    fn contract_icon_uri(&self) -> Option<String>;

    /// Returns a URI (e.g., HTTPS, IPFS) pointing to the project's main website or documentation.
    /// Returns `None` if not set.
    fn contract_project_uri(&self) -> Option<String>;
}

/// URI pointing to the project's website or documentation.
fn contract_project_uri(&self) -> Option<String>;
}

#[odra::module]
pub struct Cep96 {
pub contract_name: SubModule<Cep96NameStorage>,
pub contract_description: SubModule<Cep96DescriptionStorage>,
pub contract_icon_uri: SubModule<Cep96IconUriStorage>,
pub contract_project_uri: SubModule<Cep96ProjectUriStorage>
}

#[odra::module]
impl Cep96ContractMetadata for Cep96 {
fn contract_name(&self) -> Option<String> {
self.contract_name.get()
}

fn contract_description(&self) -> Option<String> {
self.contract_description.get()
}

fn contract_icon_uri(&self) -> Option<String> {
self.contract_icon_uri.get()
}

fn contract_project_uri(&self) -> Option<String> {
self.contract_project_uri.get()
}
}

impl Cep96 {
pub fn init(
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

The init method only sets values that are provided as Some. Combined with the storage modules' logic that prevents updates after the first set, this creates an initialization pattern that cannot be corrected if a mistake is made (e.g., a typo in the name). If init is called with None for a field, that field can never be initialized later via the Cep96 module's API. This is a rigid architectural choice that could lead to permanently incomplete contract metadata. The design should either allow subsequent updates (changing the storage logic) or require all fields to be provided at initialization, failing otherwise.

Code Suggestion:

Option A (Allow updates, requires changing storage `set` logic):
pub fn init(...) { /* same, but storage set() allows overwrites */ }
// Later, add an `update_metadata` method with proper access control.
Option B (Require complete initialization):
pub fn init(
    &self,
    name: String,
    description: String,
    icon_uri: String,
    project_uri: String
) {
    self.contract_name.set(name);
    self.contract_description.set(description);
    self.contract_icon_uri.set(icon_uri);
    self.contract_project_uri.set(project_uri);
}

Evidence: path:modules/src/cep96/storage.rs, method:set

&self,
name: Option<String>,
description: Option<String>,
icon_uri: Option<String>,
project_uri: Option<String>
) {
if let Some(name) = name {
self.contract_name.set(name);
}
if let Some(description) = description {
self.contract_description.set(description);
}
if let Some(icon_uri) = icon_uri {
self.contract_icon_uri.set(icon_uri);
}
if let Some(project_uri) = project_uri {
self.contract_project_uri.set(project_uri);
}
}
}
62 changes: 62 additions & 0 deletions modules/src/cep96/storage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use odra::prelude::*;

const KEY_CONTRACT_NAME: &str = "contract_name";
const KEY_CONTRACT_DESCRIPTION: &str = "contract_description";
const KEY_CONTRACT_ICON_URI: &str = "contract_icon_uri";
const KEY_CONTRACT_PROJECT_URI: &str = "contract_project_uri";

#[odra::module]
pub struct Cep96NameStorage;
impl Cep96NameStorage {
pub fn set(&self, value: String) {
if self.get().is_none() {
self.env().set_named_value(KEY_CONTRACT_NAME, value);
}
}
Comment on lines +11 to +15
Copy link

Choose a reason for hiding this comment

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

P2 | Confidence: High

The CEP-96 storage modules enforce immutability after the first set() call, ignoring subsequent attempts. This design is brittle and may lead to silent failures, especially when integrated with existing token modules like Cep95. The raw_mint method in CEP-95 (and related contracts) is likely to be called multiple times. If CEP-96 metadata were to be set or updated later (e.g., via an update_metadata function), the current set logic would fail silently, potentially confusing users and leaving the contract in an incorrect state. This inflexibility clashes with common NFT contract patterns where collection metadata may need to be corrected or updated.

Suggested change
pub fn set(&self, value: String) {
if self.get().is_none() {
self.env().set_named_value(KEY_CONTRACT_NAME, value);
}
}
pub fn set(&self, value: String) {
self.env().set_named_value(KEY_CONTRACT_NAME, value);
}

Evidence: symbol:Cep95::raw_mint, path:modules/src/cep95.rs


pub fn get(&self) -> Option<String> {
self.env().get_named_value(KEY_CONTRACT_NAME)
}
}

#[odra::module]
pub struct Cep96DescriptionStorage;
impl Cep96DescriptionStorage {
pub fn set(&self, value: String) {
if self.get().is_none() {
self.env().set_named_value(KEY_CONTRACT_DESCRIPTION, value);
}
}

pub fn get(&self) -> Option<String> {
self.env().get_named_value(KEY_CONTRACT_DESCRIPTION)
}
}

#[odra::module]
pub struct Cep96IconUriStorage;
impl Cep96IconUriStorage {
pub fn set(&self, value: String) {
if self.get().is_none() {
self.env().set_named_value(KEY_CONTRACT_ICON_URI, value);
}
}

pub fn get(&self) -> Option<String> {
self.env().get_named_value(KEY_CONTRACT_ICON_URI)
}
}

#[odra::module]
pub struct Cep96ProjectUriStorage;
impl Cep96ProjectUriStorage {
pub fn set(&self, value: String) {
if self.get().is_none() {
self.env().set_named_value(KEY_CONTRACT_PROJECT_URI, value);
}
}

pub fn get(&self) -> Option<String> {
self.env().get_named_value(KEY_CONTRACT_PROJECT_URI)
}
}
1 change: 1 addition & 0 deletions modules/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub mod access;
pub mod cep18;
pub mod cep18_token;
pub mod cep95;
pub mod cep96;
pub mod erc1155;
pub mod erc1155_receiver;
pub mod erc1155_token;
Expand Down
2 changes: 1 addition & 1 deletion odra-wasm-client-builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ convert_case = { workspace = true }
proc-macro2 = { workspace = true }
quote = { workspace = true }
serde_json = { workspace = true }
syn = { workspace = true }
syn = { workspace = true, features = ["full", "extra-traits"] }
thiserror = { workspace = true }

[dev-dependencies]
Expand Down
Loading