Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(erc20): implement ERC20Burnable extension #31

Merged
merged 19 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 16 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions contracts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ default = []
tests = []
erc20 = []
erc20_metadata = ["erc20"]
erc20_burnable = ["erc20"]
erc721 = []

[lib]
Expand Down
192 changes: 192 additions & 0 deletions contracts/src/erc20/extensions/burnable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
//! Optional Burnable extension of the ERC-20 standard.

#[macro_export]
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this to just above macro_rules!. The rationale is:

  • Consistency with the way we declare the attributes in the rest of the crate.
  • This is preferred in Rust (check any item in std or any official crate like regex)
  • It's better to keep attributes close to what they decorate so that we don't have to jump around the code (also it's easy to miss them when they're like this).

/// This macro provides implementation of ERC-20 Burnable extension.
///
/// It adds `burn` and `burn_from` function
/// to a custom token that contains `ERC20 erc20` attribute.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// This macro provides implementation of ERC-20 Burnable extension.
///
/// It adds `burn` and `burn_from` function
/// to a custom token that contains `ERC20 erc20` attribute.
/// This macro provides an implementation of the ERC-20 Burnable extension.
///
/// It adds the `burn` and `burn_from` functions, and expects the token
/// to contain `ERC20 erc20` as a field. See [`crate::ERC20`].

///
/// Requires import of:
/// * alloy_primitives::{Address, U256}

Check warning on line 10 in contracts/src/erc20/extensions/burnable.rs

View workflow job for this annotation

GitHub Actions / clippy

[clippy] contracts/src/erc20/extensions/burnable.rs#L10

warning: item in documentation is missing backticks --> contracts/src/erc20/extensions/burnable.rs:10:7 | 10 | /// * alloy_primitives::{Address, U256} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown help: try | 10 | /// * `alloy_primitives::{Address`, U256} | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Raw output
contracts/src/erc20/extensions/burnable.rs:10:7:w:warning: item in documentation is missing backticks
  --> contracts/src/erc20/extensions/burnable.rs:10:7
   |
10 | /// * alloy_primitives::{Address, U256}
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
help: try
   |
10 | /// * `alloy_primitives::{Address`, U256}
   |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~


__END__
Copy link
Contributor

@alexfertel alexfertel Apr 9, 2024

Choose a reason for hiding this comment

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

We should use fully qualified paths inside the macro instead of keeping this as a comment. I'll make the suggestions in the following comments.

/// * contracts::erc20::{Error, ERC20}

Check warning on line 11 in contracts/src/erc20/extensions/burnable.rs

View workflow job for this annotation

GitHub Actions / clippy

[clippy] contracts/src/erc20/extensions/burnable.rs#L11

warning: item in documentation is missing backticks --> contracts/src/erc20/extensions/burnable.rs:11:7 | 11 | /// * contracts::erc20::{Error, ERC20} | ^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown help: try | 11 | /// * `contracts::erc20::{Error`, ERC20} | ~~~~~~~~~~~~~~~~~~~~~~~~~~
Raw output
contracts/src/erc20/extensions/burnable.rs:11:7:w:warning: item in documentation is missing backticks
  --> contracts/src/erc20/extensions/burnable.rs:11:7
   |
11 | /// * contracts::erc20::{Error, ERC20}
   |       ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
help: try
   |
11 | /// * `contracts::erc20::{Error`, ERC20}
   |       ~~~~~~~~~~~~~~~~~~~~~~~~~~


__END__
/// * stylus_sdk::msg

Check warning on line 12 in contracts/src/erc20/extensions/burnable.rs

View workflow job for this annotation

GitHub Actions / clippy

[clippy] contracts/src/erc20/extensions/burnable.rs#L12

warning: item in documentation is missing backticks --> contracts/src/erc20/extensions/burnable.rs:12:7 | 12 | /// * stylus_sdk::msg | ^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown help: try | 12 | /// * `stylus_sdk::msg` | ~~~~~~~~~~~~~~~~~
Raw output
contracts/src/erc20/extensions/burnable.rs:12:7:w:warning: item in documentation is missing backticks
  --> contracts/src/erc20/extensions/burnable.rs:12:7
   |
12 | /// * stylus_sdk::msg
   |       ^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
help: try
   |
12 | /// * `stylus_sdk::msg`
   |       ~~~~~~~~~~~~~~~~~


__END__
macro_rules! impl_erc20_burnable {

Check warning on line 13 in contracts/src/erc20/extensions/burnable.rs

View workflow job for this annotation

GitHub Actions / clippy

[clippy] contracts/src/erc20/extensions/burnable.rs#L13

warning: item name ends with its containing module's name --> contracts/src/erc20/extensions/burnable.rs:13:14 | 13 | macro_rules! impl_erc20_burnable { | ^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions = note: `#[warn(clippy::module_name_repetitions)]` implied by `#[warn(clippy::pedantic)]`
Raw output
contracts/src/erc20/extensions/burnable.rs:13:14:w:warning: item name ends with its containing module's name
  --> contracts/src/erc20/extensions/burnable.rs:13:14
   |
13 | macro_rules! impl_erc20_burnable {
   |              ^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
   = note: `#[warn(clippy::module_name_repetitions)]` implied by `#[warn(clippy::pedantic)]`


__END__
() => {
/// Destroys a `value` amount of tokens from the caller.
/// lowering the total supply.
///
/// Relies on the `update` mechanism.
///
/// # Arguments
///
/// * `value` - Amount to be burnt.
///
/// # Errors
///
/// If the `from` address doesn't have enough tokens, then the error
/// [`Error::InsufficientBalance`] is returned.
///
/// # Events
///
/// Emits a [`Transfer`] event.
pub(crate) fn burn(&mut self, value: U256) -> Result<(), Error> {
self.erc20._burn(msg::sender(), value)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub(crate) fn burn(&mut self, value: U256) -> Result<(), Error> {
self.erc20._burn(msg::sender(), value)
}
pub fn burn(&mut self, value: alloy_primitives::U256) -> Result<(), contracts::erc20::Error> {
self.erc20._burn(stylus_sdk::msg::sender(), value)
}


/// Destroys a `value` amount of tokens from `account`,
/// lowering the total supply.
///
/// Relies on the `update` mechanism.
///
/// # Arguments
///
/// * `account` - Owner's address.
/// * `value` - Amount to be burnt.
///
/// # Errors
///
/// If not enough allowance is available, then the error
/// [`Error::InsufficientAllowance`] is returned.
/// * If the `from` address is `Address::ZERO`, then the error
/// [`Error::InvalidSender`] is returned.
/// If the `from` address doesn't have enough tokens, then the error
/// [`Error::InsufficientBalance`] is returned.
///
/// # Events
///
/// Emits a [`Transfer`] event.
pub(crate) fn burn_from(
&mut self,
account: Address,
value: U256,
) -> Result<(), Error> {
self.erc20._spend_allowance(account, msg::sender(), value)?;
self.erc20._burn(account, value)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub(crate) fn burn_from(
&mut self,
account: Address,
value: U256,
) -> Result<(), Error> {
self.erc20._spend_allowance(account, msg::sender(), value)?;
self.erc20._burn(account, value)
}
pub fn burn_from(
&mut self,
account: alloy_primitives::Address,
value: alloy_primitives::U256,
) -> Result<(), contracts::erc20::Error> {
self.erc20._spend_allowance(account, stylus_sdk::msg::sender(), value)?;
self.erc20._burn(account, value)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a problem with contracts::erc20::Error -> we cannot have it this way, while it breaks all imports in tests...

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I feared that...not sure what the solution is 😢

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, fixed it!

        pub(crate) fn burn_from(
            &mut self,
            account: alloy_primitives::Address,
            value: alloy_primitives::U256,
        ) -> Result<(), alloc::vec::Vec<u8>> {
            self.erc20._spend_allowance(
                account,
                stylus_sdk::msg::sender(),
                value,
            )?;
            self.erc20._burn(account, value).map_err(|e| e.into())
        }

};
}

#[cfg(test)]
mod tests {
use alloy_primitives::{address, Address, U256};
use stylus_sdk::{msg, prelude::*};

use crate::erc20::{Error, ERC20};

sol_storage! {
pub struct TestERC20Burnable {
ERC20 erc20;
}
}

#[external]
#[inherit(ERC20)]
impl TestERC20Burnable {
impl_erc20_burnable!();
}

impl Default for TestERC20Burnable {
fn default() -> Self {
Self { erc20: ERC20::default() }
}
}

#[grip::test]
fn burns(contract: TestERC20Burnable) {
let zero = U256::ZERO;
let one = U256::from(1);

assert_eq!(zero, contract.erc20.total_supply());

// Mint some tokens for msg::sender().
let sender = msg::sender();

let two = U256::from(2);
contract.erc20._update(Address::ZERO, sender, two).unwrap();
assert_eq!(two, contract.erc20.balance_of(sender));
assert_eq!(two, contract.erc20.total_supply());

contract.burn(one).unwrap();

assert_eq!(one, contract.erc20.balance_of(sender));
assert_eq!(one, contract.erc20.total_supply());
}

#[grip::test]
fn burns_errors_when_insufficient_balance(contract: TestERC20Burnable) {
let one = U256::from(1);
let sender = msg::sender();

assert_eq!(U256::ZERO, contract.erc20.balance_of(sender));

let result = contract.burn(one);

assert!(matches!(result, Err(Error::InsufficientBalance(_))));
}

#[grip::test]
fn burn_from(contract: TestERC20Burnable) {
let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");
let sender = msg::sender();

// Alice approves `msg::sender`.
let one = U256::from(1);
contract.erc20._allowances.setter(alice).setter(sender).set(one);

// Mint some tokens for Alice.
let two = U256::from(2);
contract.erc20._update(Address::ZERO, alice, two).unwrap();
assert_eq!(two, contract.erc20.balance_of(alice));
assert_eq!(two, contract.erc20.total_supply());

contract.burn_from(alice, one).unwrap();

assert_eq!(one, contract.erc20.balance_of(alice));
assert_eq!(one, contract.erc20.total_supply());
assert_eq!(U256::ZERO, contract.erc20.allowance(alice, sender));
}

#[grip::test]
fn burns_from_errors_when_insufficient_balance(
contract: TestERC20Burnable,
) {
let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");

// Alice approves `msg::sender`.
let one = U256::from(1);
contract.erc20._allowances.setter(alice).setter(msg::sender()).set(one);
assert_eq!(U256::ZERO, contract.erc20.balance_of(alice));

let one = U256::from(1);
let result = contract.burn_from(alice, one);
assert!(matches!(result, Err(Error::InsufficientBalance(_))));
}

#[grip::test]
fn burns_from_errors_when_invalid_sender(contract: TestERC20Burnable) {
let one = U256::from(1);
contract
.erc20
._allowances
.setter(Address::ZERO)
.setter(msg::sender())
.set(one);
let result = contract.burn_from(Address::ZERO, one);
assert!(matches!(result, Err(Error::InvalidSender(_))));
}

#[grip::test]
fn burns_from_errors_when_insufficient_allowance(
contract: TestERC20Burnable,
) {
let alice = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");

// Mint some tokens for Alice.
let one = U256::from(1);
contract.erc20._update(Address::ZERO, alice, one).unwrap();
assert_eq!(one, contract.erc20.balance_of(alice));

let result = contract.burn_from(alice, one);
assert!(matches!(result, Err(Error::InsufficientAllowance(_))));
}
}
5 changes: 5 additions & 0 deletions contracts/src/erc20/extensions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ cfg_if::cfg_if! {
pub use metadata::Metadata;
}
}
cfg_if::cfg_if! {
if #[cfg(any(test, feature = "erc20_burnable"))] {
pub mod burnable;
}
}
Loading
Loading