Add set_name, set_decimals and set_symbol to cep18

[zie1ony]

No description provided.

[llamapreview]

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Request Changes

This PR introduces mutable metadata functions to the CEP-18 token without access control, posing security risks and breaking standard compliance by allowing decimal changes that corrupt financial integrity.

📄 Documentation Diagram

This diagram documents the new workflow for setting CEP-18 token metadata.

sequenceDiagram
    participant Caller as External Caller
    participant CEP18 as CEP-18 Token
    participant Storage as Contract Storage
    Caller->>CEP18: Call set_name(name)
    note over CEP18: PR #35;630 adds set_name, set_symbol, set_decimals functions
    CEP18->>Storage: Update name
    Caller->>CEP18: Call set_symbol(symbol)
    CEP18->>Storage: Update symbol
    Caller->>CEP18: Call set_decimals(decimals)
    CEP18->>Storage: Update decimals

Loading

Priority	File	Category	Impact Summary	Anchors
P1	modules/src/cep18_token.rs	Security	No access control enables unauthorized metadata changes leading to spoofing.
P1	modules/src/cep18_token.rs	Architecture	Mutable decimals breaks token standards and corrupts balance calculations.
P2	modules/src/cep18_token.rs	Maintainability	Missing validation for decimals could cause integration issues.
P2	modules/src/cep18_token.rs	Maintainability	No length validation for name may bloat storage.
P2	CHANGELOG.md	Documentation	Changelog lacks warnings for security and behavioral changes.

🔍 Notable Themes

• Access Control Gap: All new mutable functions lack any authorization checks, creating a systemic security vulnerability that allows arbitrary token metadata changes.
• Standard Violation: Introducing mutable decimals deviates from common token standards (like ERC-20), risking integration failures and irreversible financial corruption.

📈 Risk Diagram

This diagram illustrates the security and architectural risks introduced by mutable token metadata without access control.

sequenceDiagram
    participant Attacker as Malicious Actor
    participant CEP18 as CEP-18 Token
    participant Storage as Contract Storage
    Attacker->>CEP18: Call set_name("Fake Token")
    note over CEP18: R1(P1): No access control allows token spoofing
    CEP18->>Storage: Update name to "Fake Token"
    Attacker->>CEP18: Call set_decimals(new_value)
    note over CEP18: R2(P1): Mutable decimals corrupts financial integrity
    CEP18->>Storage: Update decimals

Loading

---

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

[llamapreview]

P1 | Confidence: High

• The new public functions directly mutate core token metadata (name, symbol, decimals) without any access control. This is a severe security vulnerability. Any arbitrary external caller can change the token's identity and decimal precision at any time. This can be exploited to: 1. Spoof tokens: An attacker can rename the token to impersonate a legitimate asset, leading to phishing and user confusion. 2. Break financial logic: Changing decimals after deployment can corrupt all existing balances and calculations, as token amounts are stored in base units (e.g., U256). For example, if a token has 100 base units and decimals changes from 2 to 3, the human-readable value changes from 1.00 to 0.100, effectively stealing 90% of each holder's value. This is an irreversible, protocol-breaking change with direct evidence in the snippet.
• Adding a mutable decimals field fundamentally changes the token's architectural guarantees and violates the principle of least surprise for CEP-18 (a Casper analog of ERC-20). In standard token implementations, decimals is immutable after deployment because: 1. Financial integrity: All on-chain calculations (balances, allowances, transfers) and off-chain integrations (wallets, exchanges) assume a fixed decimal precision. Changing it corrupts the token's arithmetic without migrating balances, a catastrophic failure. 2. Standard compliance: While CEP-18 may not explicitly forbid mutability, prominent standards (ERC-20, BEP-20) treat decimals as immutable, and changing this is a significant, breaking behavioral change for the module. Existing integrations will break if decimals can change. The direct code change introduces this breaking architectural risk with no safeguards (see also the security finding for lack of access control). The impact is high-probability data corruption.
• The set_decimals function accepts any u8 value (0-255) without validation. While u8 is technically correct, extreme values can cause integration issues or runtime errors in downstream logic (e.g., UI rendering, decimal formatting libraries). Although not a direct runtime crash in the contract, allowing nonsensical values (e.g., decimals = 255) reduces robustness and increases support burden.
• The function accepts a String without any length validation or sanitization. While not a security issue in the provided snippet (assuming storage handles arbitrary data), excessively long names could bloat storage or cause issues in off-chain tooling that expects reasonable lengths. This is a minor maintainability concern.

Code Suggestion:

// Ensure the contract inherits or uses an access control mechanism (e.g., Ownable).
// Example assumes `self` has an `only_owner` guard or similar.
pub fn set_name(&mut self, name: String) {
    self.ensure_owner(); // Or equivalent access control check
    const MAX_NAME_LENGTH: usize = 50; // Example reasonable limit
    if name.len() > MAX_NAME_LENGTH {
        self.env().revert(Error::NameTooLong);
    }
    self.name.set(name);
}

// Apply same pattern to set_symbol and set_decimals
pub fn set_decimals(&mut self, decimals: u8) {
    self.ensure_owner();
    const MAX_DECIMALS: u8 = 18; // Assuming a reasonable max
    if decimals > MAX_DECIMALS {
        self.env().revert(Error::InvalidDecimals); // Define appropriate error
    }
    self.decimals.set(decimals);
}

[llamapreview]

P2 | Confidence: High

The changelog entry accurately describes the addition but does not warn about the critical behavioral change and security implications. For a public-facing module like CEP-18, changelogs should highlight breaking changes or security requirements. Since these functions introduce mutable metadata (a significant deviation from common token standards) and lack access control, the entry should include a warning or reference to necessary access control setup.

Suggested change

	### Added
	- `set_name`, `set_symbol` and `set_decimals` functions to `CEP-18` module.
	### Added
	- `set_name`, `set_symbol` and `set_decimals` functions to `CEP-18` module.
	**Warning:** These functions are public by default. To prevent unauthorized changes, ensure the contract uses an access control mechanism (e.g., `Ownable`). Changing `decimals` after deployment can corrupt token accounting.