Skip to content

Commit

Permalink
Remove non-standard increase_allowance and decrease_allowance from ER…
Browse files Browse the repository at this point in the history
…C20 (#881)

* remove_increase_and_decrease_allowance_erc20

* erc20_remove_increase_decrease_allowance

* update

* update

* changelog-entry

* changelog-update

* changelog-update

* update-class-hash

* update-class-hashes

* update

* update

* update-docs

* changelog
  • Loading branch information
TAdev0 authored Feb 6, 2024
1 parent a34025c commit 2815498
Show file tree
Hide file tree
Showing 12 changed files with 8 additions and 477 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Bump scarb to v2.4.4 (#853)
- Bump scarb to v2.5.3 (#898)

### Removed

- Non standard increase_allowance and decrease_allowance functions in ERC20 contract (#881)

## 0.8.1 (2024-01-23)

Expand All @@ -36,3 +40,4 @@ 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)

71 changes: 0 additions & 71 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.

[.contract-item]
[[ERC20Component-_spend_allowance]]
==== `[.contract-item-name]#++_spend_allowance++#++(ref self: ContractState, owner: ContractAddress, spender: ContractAddress, amount: u256)++` [.item-kind]#internal#
Expand Down Expand Up @@ -505,9 +436,7 @@ include::../utils/_class_hashes.adoc[]

* xref:#ERC20Component-Embeddable-Impls[`++ERC20Impl++`]
* xref:#ERC20Component-Embeddable-Impls[`++ERC20MetadataImpl++`]
* xref:#ERC20Component-Embeddable-Impls[`++SafeAllowanceImpl++`]
* xref:#ERC20Component-Embeddable-Impls-camelCase[`++ERC20CamelOnlyImpl++`]
* xref:#ERC20Component-Embeddable-Impls-camelCase[`++SafeAllowanceCamelImpl++`]
--

[#ERC20-constructor-section]
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 as well as `IERC20Camel` for backwards compatibility.
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
5 changes: 0 additions & 5 deletions docs/modules/ROOT/pages/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,7 @@ mod MyERC20Token {
#[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
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. This is useful to interface with a preset contract offered by this library, such as the ERC20 preset that includes functions from different traits such as `IERC20` and `IERC20Camel`.

```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: 0x01148c31dfa5c4708a4e9cf1eb0fd3d4d8ad9ccf09d0232cd6b56bee64a7de9d
:eth-account-upgradeable-class-hash: 0x023e416842ca96b1f7067693892ed00881d97a4b0d9a4c793b75cb887944d98d
:erc20-class-hash: 0x03dcc315f11192ff18648dcd8306f324aa7cf11f2f001ef9fd53afe730efb71b
:erc20-class-hash: 0x07c9b5a246fe83b0cd81de65daddb94b3803f94950db99bf2431bbfecf284642
:erc721-class-hash: 0x06b7c9efc5467c621f58d87995302d940a39b7217b5c5a7a55555c97cabf5cd8

// 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 @@ -7,7 +7,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 @@ -291,134 +290,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
Loading

0 comments on commit 2815498

Please sign in to comment.