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

Remove non-standard increase_allowance and decrease_allowance from ERC20 #881

Merged
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
scarb 2.4.4
TAdev0 marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Support higher tx versions in Account (#858)
- Bump scarb to v2.4.1 (#858)
- Add security section to Upgrades docs (#861)

### Removed

- Non standard increase_allowance and decrease_allowance functions in ERC20 contract (#881)
TAdev0 marked this conversation as resolved.
Show resolved Hide resolved
69 changes: 0 additions & 69 deletions docs/modules/ROOT/pages/api/erc20.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,6 @@ ERC20 component extending <<IERC20,IERC20>> and <<IERC20Metadata,IERC20Metadata>
* xref:#ERC20Component-name[`++name(self)++`]
* xref:#ERC20Component-symbol[`++symbol(self)++`]
* xref:#ERC20Component-decimals[`++decimals(self)++`]

.SafeAllowanceImpl
* xref:#ERC20Component-increase_allowance[`++increase_allowance(self, spender, added_value)++`]
* xref:#ERC20Component-decrease_allowance[`++decrease_allowance(self, spender, subtracted_value)++`]
--

[.contract-index#ERC20Component-Embeddable-Impls-camelCase]
Expand All @@ -196,10 +192,6 @@ ERC20 component extending <<IERC20,IERC20>> and <<IERC20Metadata,IERC20Metadata>
* xref:#ERC20Component-totalSupply[`++totalSupply(self)++`]
* xref:#ERC20Component-balanceOf[`++balanceOf(self, account)++`]
* xref:#ERC20Component-transferFrom[`++transferFrom(self, sender, recipient, amount)++`]

.SafeAllowanceCamelImpl
* xref:#ERC20Component-increaseAllowance[`++increaseAllowance(self, spender, addedValue)++`]
* xref:#ERC20Component-decreaseAllowance[`++decreaseAllowance(self, spender, subtractedValue)++`]
--

[.contract-index]
Expand All @@ -211,8 +203,6 @@ ERC20 component extending <<IERC20,IERC20>> and <<IERC20Metadata,IERC20Metadata>
* xref:#ERC20Component-_approve[`++_approve(self, owner, spender, amount)++`]
* xref:#ERC20Component-_mint[`++_mint(self, recipient, amount)++`]
* xref:#ERC20Component-_burn[`++_burn(self, account, amount)++`]
* xref:#ERC20Component-_increase_allowance[`++_increase_allowance(self, spender, added_value)++`]
* xref:#ERC20Component-_decrease_allowance[`++_decrease_allowance(self, spender, subtracted_value)++`]
* xref:#ERC20Component-_spend_allowance[`++_spend_allowance(self, owner, spender, amount)++`]
--

Expand Down Expand Up @@ -296,33 +286,6 @@ See <<IERC20Metadata-symbol,IERC20Metadata::symbol>>.

See <<IERC20Metadata-decimals,IERC20Metadata::decimals>>.

[.contract-item]
[[ERC20Component-increase_allowance]]
==== `[.contract-item-name]#++increase_allowance++#++(ref self: ContractState, spender: ContractAddress, added_value: u256) → bool++` [.item-kind]#external#

Increases the allowance granted from the caller to `spender` by `added_value`
Returns `true` on success, reverts otherwise.

Emits an <<ERC20Component-Approval,Approval>> event.

Requirements:

- `spender` cannot be the zero address.

[.contract-item]
[[ERC20Component-decrease_allowance]]
==== `[.contract-item-name]#++decrease_allowance++#++(ref self: ContractState, spender: ContractAddress, subtracted_value: u256) → bool++` [.item-kind]#external#

Decreases the allowance granted from the caller to `spender` by `subtracted_value`
Returns `true` on success.

Emits an <<ERC20Component-Approval,Approval>> event.

Requirements:

- `spender` cannot be the zero address.
- `spender` must have allowance for the caller of at least `subtracted_value`.

[#ERC20Component-camelCase-support]
==== camelCase Support

Expand Down Expand Up @@ -350,22 +313,6 @@ See <<IERC20-transfer_from,IERC20::transfer_from>>.

Supports the Cairo v0 convention of writing external methods in camelCase as discussed {casing-discussion}.

[.contract-item]
[[ERC20Component-increaseAllowance]]
==== `[.contract-item-name]#++increaseAllowance++#++(ref self: ContractState, spender: ContractAddress, addedValue: u256) → bool++` [.item-kind]#external#

See <<ERC20Component-increase_allowance,increase_allowance>>.

Supports the Cairo v0 convention of writing external methods in camelCase as discussed {casing-discussion}.

[.contract-item]
[[ERC20Component-decreaseAllowance]]
==== `[.contract-item-name]#++decreaseAllowance++#++(ref self: ContractState, spender: ContractAddress, subtractedValue: u256) → bool++` [.item-kind]#external#

See <<ERC20Component-decrease_allowance,decrease_allowance>>.

Supports the Cairo v0 convention of writing external methods in camelCase as discussed {casing-discussion}.

[#ERC20Component-Internal-functions]
==== Internal functions

Expand Down Expand Up @@ -431,22 +378,6 @@ Requirements:

- `account` cannot be the zero address.

[.contract-item]
[[ERC20Component-_increase_allowance]]
==== `[.contract-item-name]#++_increase_allowance++#++(ref self: ContractState, spender: ContractAddress, added_value: u256)++` [.item-kind]#internal#

Increases the allowance granted from the caller to `spender` by `added_value`

Emits an <<ERC20Component-Approval,Approval>> event.

[.contract-item]
[[ERC20Component-_decrease_allowance]]
==== `[.contract-item-name]#++_decrease_allowance++#++(ref self: ContractState, spender: ContractAddress, subtracted_value: u256)++` [.item-kind]#internal#

Decreases the allowance granted from the caller to `spender` by `subtracted_value`

Emits an <<ERC20Component-Approval,Approval>> event.

TAdev0 marked this conversation as resolved.
Show resolved Hide resolved
[.contract-item]
[[ERC20Component-_spend_allowance]]
==== `[.contract-item-name]#++_spend_allowance++#++(ref self: ContractState, owner: ContractAddress, spender: ContractAddress, amount: u256)++` [.item-kind]#internal#
Expand Down
10 changes: 1 addition & 9 deletions docs/modules/ROOT/pages/erc20.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ See the {custom-decimals} guide.
:erc20-supply: xref:/guides/erc20-supply.adoc[Creating ERC20 Supply]

The following interface represents the full ABI of the Contracts for Cairo `ERC20` component.
The interface includes the `IERC20` standard interface as well as non-standard functions like `increase_allowance`.
The interface includes the `IERC20` standard interface.
TAdev0 marked this conversation as resolved.
Show resolved Hide resolved
To support older token deployments, as mentioned in {dual-interfaces}, the `ERC20` component also includes an implementation of the interface written in camelCase.

[,javascript]
Expand All @@ -39,20 +39,12 @@ trait ERC20ABI {
fn symbol() -> felt252;
fn decimals() -> u8;

// ISafeAllowance
fn increase_allowance(spender: ContractAddress, added_value: u256) -> bool;
fn decrease_allowance(spender: ContractAddress, subtracted_value: u256) -> bool;

// IERC20Camel
fn totalSupply() -> u256;
fn balanceOf(account: ContractAddress) -> u256;
fn transferFrom(
sender: ContractAddress, recipient: ContractAddress, amount: u256
) -> bool;

// ISafeAllowanceCamel
fn increaseAllowance(spender: ContractAddress, addedValue: u256) -> bool;
fn decreaseAllowance(spender: ContractAddress, subtractedValue: u256) -> bool;
}
----

Expand Down
6 changes: 1 addition & 5 deletions docs/modules/ROOT/pages/interfaces.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ trait IERC20<TState> {

=== ABI traits

They describe a contract's complete interface. This is useful to interface with a preset contract offered by this library, such as the ERC20 preset that includes non-standard functions like `increase_allowance`.
They describe a contract's complete interface.
TAdev0 marked this conversation as resolved.
Show resolved Hide resolved
TAdev0 marked this conversation as resolved.
Show resolved Hide resolved

```javascript
#[starknet::interface]
Expand All @@ -61,10 +61,6 @@ trait ERC20ABI<TState> {
ref self: TState, sender: ContractAddress, recipient: ContractAddress, amount: u256
) -> bool;
fn approve(ref self: TState, spender: ContractAddress, amount: u256) -> bool;
fn increase_allowance(ref self: TState, spender: ContractAddress, added_value: u256) -> bool;
fn decrease_allowance(
ref self: TState, spender: ContractAddress, subtracted_value: u256
) -> bool;
}
```

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/utils/_class_hashes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// Class Hashes
:account-class-hash: 0x0402765bcede84b1267a9d4658a7737c3c41a8caf6201984c3df95577c2298a3
:eth-account-upgradeable-class-hash: 0x03dda9bcfa854795d91d586b1a4275a68ab1ab185b33a5c00ce647c75875b0ff
:erc20-class-hash: 0x03af5816946625d3d2c94ea451225715784762050eba736f0b0ad9186685bced
:erc20-class-hash: 0x7c9b5a246fe83b0cd81de65daddb94b3803f94950db99bf2431bbfecf284642
TAdev0 marked this conversation as resolved.
Show resolved Hide resolved
:erc721-class-hash: 0x045c96d1b24c3dc060680e4bfd4bdc32161aefe8f8939cd4be3954c5d8688d75

// Presets page
Expand Down
5 changes: 0 additions & 5 deletions src/presets/erc20.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ mod ERC20 {
#[abi(embed_v0)]
impl ERC20MetadataImpl = ERC20Component::ERC20MetadataImpl<ContractState>;
#[abi(embed_v0)]
impl SafeAllowanceImpl = ERC20Component::SafeAllowanceImpl<ContractState>;
#[abi(embed_v0)]
impl ERC20CamelOnlyImpl = ERC20Component::ERC20CamelOnlyImpl<ContractState>;
#[abi(embed_v0)]
impl SafeAllowanceCamelImpl =
ERC20Component::SafeAllowanceCamelImpl<ContractState>;
impl InternalImpl = ERC20Component::InternalImpl<ContractState>;

#[storage]
Expand Down
10 changes: 0 additions & 10 deletions src/tests/mocks/erc20_mocks.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ mod DualCaseERC20Mock {
#[abi(embed_v0)]
impl ERC20MetadataImpl = ERC20Component::ERC20MetadataImpl<ContractState>;
#[abi(embed_v0)]
impl SafeAllowanceImpl = ERC20Component::SafeAllowanceImpl<ContractState>;
#[abi(embed_v0)]
impl ERC20CamelOnlyImpl = ERC20Component::ERC20CamelOnlyImpl<ContractState>;
#[abi(embed_v0)]
impl SafeAllowanceCamelImpl =
ERC20Component::SafeAllowanceCamelImpl<ContractState>;
impl InternalImpl = ERC20Component::InternalImpl<ContractState>;

#[storage]
Expand Down Expand Up @@ -55,8 +50,6 @@ mod SnakeERC20Mock {
impl ERC20Impl = ERC20Component::ERC20Impl<ContractState>;
#[abi(embed_v0)]
impl ERC20MetadataImpl = ERC20Component::ERC20MetadataImpl<ContractState>;
#[abi(embed_v0)]
impl SafeAllowanceImpl = ERC20Component::SafeAllowanceImpl<ContractState>;
impl InternalImpl = ERC20Component::InternalImpl<ContractState>;

#[storage]
Expand Down Expand Up @@ -96,9 +89,6 @@ mod CamelERC20Mock {
impl ERC20MetadataImpl = ERC20Component::ERC20MetadataImpl<ContractState>;
#[abi(embed_v0)]
impl ERC20CamelOnlyImpl = ERC20Component::ERC20CamelOnlyImpl<ContractState>;
#[abi(embed_v0)]
impl SafeAllowanceCamelImpl =
ERC20Component::SafeAllowanceCamelImpl<ContractState>;

// `ERC20Impl` is not embedded because it would defeat the purpose of the
// mock. The `ERC20Impl` case-agnostic methods are manually exposed.
Expand Down
129 changes: 0 additions & 129 deletions src/tests/presets/test_erc20.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use openzeppelin::tests::utils;
use openzeppelin::token::erc20::ERC20Component::{Approval, Transfer};
use openzeppelin::token::erc20::ERC20Component::{ERC20CamelOnlyImpl, ERC20Impl};
use openzeppelin::token::erc20::ERC20Component::{ERC20MetadataImpl, InternalImpl};
use openzeppelin::token::erc20::ERC20Component::{SafeAllowanceImpl, SafeAllowanceCamelImpl};
use openzeppelin::token::erc20::interface::ERC20ABI;
use openzeppelin::token::erc20::interface::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait};
use openzeppelin::utils::serde::SerializedAppend;
Expand Down Expand Up @@ -292,134 +291,6 @@ fn test_transferFrom_from_zero_address() {
dispatcher.transferFrom(Zeroable::zero(), RECIPIENT(), VALUE);
}

//
// increase_allowance & increaseAllowance
//

#[test]
fn test_increase_allowance() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.approve(SPENDER(), VALUE);
utils::drop_event(dispatcher.contract_address);

assert!(dispatcher.increase_allowance(SPENDER(), VALUE));

assert_only_event_approval(dispatcher.contract_address, OWNER(), SPENDER(), VALUE * 2);

let allowance = dispatcher.allowance(OWNER(), SPENDER());
assert_eq!(allowance, VALUE * 2);
}

#[test]
#[should_panic(expected: ('ERC20: approve to 0', 'ENTRYPOINT_FAILED'))]
fn test_increase_allowance_to_zero_address() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.increase_allowance(Zeroable::zero(), VALUE);
}

#[test]
#[should_panic(expected: ('ERC20: approve from 0', 'ENTRYPOINT_FAILED'))]
fn test_increase_allowance_from_zero_address() {
let mut dispatcher = setup_dispatcher();
dispatcher.increase_allowance(SPENDER(), VALUE);
}

#[test]
fn test_increaseAllowance() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.approve(SPENDER(), VALUE);
utils::drop_event(dispatcher.contract_address);

assert!(dispatcher.increaseAllowance(SPENDER(), VALUE));

assert_only_event_approval(dispatcher.contract_address, OWNER(), SPENDER(), 2 * VALUE);

let allowance = dispatcher.allowance(OWNER(), SPENDER());
assert_eq!(allowance, VALUE * 2);
}

#[test]
#[should_panic(expected: ('ERC20: approve to 0', 'ENTRYPOINT_FAILED'))]
fn test_increaseAllowance_to_zero_address() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.increaseAllowance(Zeroable::zero(), VALUE);
}

#[test]
#[should_panic(expected: ('ERC20: approve from 0', 'ENTRYPOINT_FAILED'))]
fn test_increaseAllowance_from_zero_address() {
let mut dispatcher = setup_dispatcher();
dispatcher.increaseAllowance(SPENDER(), VALUE);
}

//
// decrease_allowance & decreaseAllowance
//

#[test]
fn test_decrease_allowance() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.approve(SPENDER(), VALUE);
utils::drop_event(dispatcher.contract_address);

assert!(dispatcher.decrease_allowance(SPENDER(), VALUE));

assert_only_event_approval(dispatcher.contract_address, OWNER(), SPENDER(), 0);

let allowance = dispatcher.allowance(OWNER(), SPENDER());
assert!(allowance.is_zero());
}

#[test]
#[should_panic(expected: ('u256_sub Overflow', 'ENTRYPOINT_FAILED'))]
fn test_decrease_allowance_to_zero_address() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.decrease_allowance(Zeroable::zero(), VALUE);
}

#[test]
#[should_panic(expected: ('u256_sub Overflow', 'ENTRYPOINT_FAILED'))]
fn test_decrease_allowance_from_zero_address() {
let mut dispatcher = setup_dispatcher();
dispatcher.decrease_allowance(SPENDER(), VALUE);
}

#[test]
fn test_decreaseAllowance() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.approve(SPENDER(), VALUE);
utils::drop_event(dispatcher.contract_address);

assert!(dispatcher.decreaseAllowance(SPENDER(), VALUE));

assert_only_event_approval(dispatcher.contract_address, OWNER(), SPENDER(), 0);

let allowance = dispatcher.allowance(OWNER(), SPENDER());
assert!(allowance.is_zero());
}

#[test]
#[should_panic(expected: ('u256_sub Overflow', 'ENTRYPOINT_FAILED'))]
fn test_decreaseAllowance_to_zero_address() {
let mut dispatcher = setup_dispatcher();
testing::set_contract_address(OWNER());
dispatcher.decreaseAllowance(Zeroable::zero(), VALUE);
}

#[test]
#[should_panic(expected: ('u256_sub Overflow', 'ENTRYPOINT_FAILED'))]
fn test_decreaseAllowance_from_zero_address() {
let mut dispatcher = setup_dispatcher();
dispatcher.decreaseAllowance(SPENDER(), VALUE);
}

//
// Helpers
//
Expand Down