Skip to content

Conversation

@MohammadNassar1
Copy link
Collaborator

@MohammadNassar1 MohammadNassar1 commented Sep 1, 2025

This change is Reviewable

@MohammadNassar1 MohammadNassar1 self-assigned this Sep 1, 2025
@MohammadNassar1 MohammadNassar1 force-pushed the mohammad/payments/add-address-whitelist branch from 9a2a6cc to 91c92c4 Compare September 1, 2025 14:25
Copy link
Collaborator

@ishay-starkware ishay-starkware left a comment

Choose a reason for hiding this comment

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

rename the title to add allowlist

@ishay-starkware reviewed all commit messages.
Reviewable status: 0 of 5 files reviewed, 8 unresolved discussions (waiting on @ImmanuelSegol)


src/payments.cairo line 91 at r1 (raw file):

        tokens: Map<ContractAddress, bool>,
        // Whitelisted addresses.
        whitelisted_addresses: Map<ContractAddress, bool>,

Suggestion:

        allowlist: Map<ContractAddress, bool>,

src/payments.cairo line 292 at r1 (raw file):

        // These functions are used by the contract to control who is allowed to take part in order
        // matching and settlement. Only whitelisted addresses are permitted to trade.

Suggestion:

allowed

src/interface.cairo line 21 at r1 (raw file):

    fn is_token_registered(self: @TContractState, token: ContractAddress) -> bool;

    fn whitelist_address(ref self: TContractState, address: ContractAddress);

Suggestion:

add_to_allowlist

src/interface.cairo line 22 at r1 (raw file):

    fn whitelist_address(ref self: TContractState, address: ContractAddress);
    fn remove_from_whitelist(ref self: TContractState, address: ContractAddress);

Suggestion:

remove_from_allowlist

src/interface.cairo line 23 at r1 (raw file):

    fn whitelist_address(ref self: TContractState, address: ContractAddress);
    fn remove_from_whitelist(ref self: TContractState, address: ContractAddress);
    fn is_whitelisted(self: @TContractState, address: ContractAddress) -> bool;

Suggestion:

is_allowed

src/errors.cairo line 3 at r1 (raw file):

use starknet::ContractAddress;

pub const ADDRESS_ALREADY_WHITELISTED: felt252 = 'ADDRESS_ALREADY_WHITELISTED';

Suggestion:

ADDRESS_ALREADY_ALLOWED

src/events.cairo line 29 at r1 (raw file):

#[derive(Debug, Drop, PartialEq, starknet::Event)]
pub struct AddressWhitelisted {

Suggestion:

AddressAllowed

src/events.cairo line 35 at r1 (raw file):

#[derive(Debug, Drop, PartialEq, starknet::Event)]
pub struct AddressRemovedFromWhitelist {

Suggestion:

AddressDisallowed

@MohammadNassar1 MohammadNassar1 force-pushed the mohammad/payments/add-address-whitelist branch from 91c92c4 to fdf9a26 Compare September 2, 2025 07:27
@MohammadNassar1 MohammadNassar1 changed the title feat(payments): add address whitelist feat(payments): add address allowlist Sep 2, 2025
Copy link
Collaborator Author

@MohammadNassar1 MohammadNassar1 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 5 files reviewed, 8 unresolved discussions (waiting on @ImmanuelSegol and @ishay-starkware)


src/events.cairo line 29 at r1 (raw file):

#[derive(Debug, Drop, PartialEq, starknet::Event)]
pub struct AddressWhitelisted {

Done.


src/events.cairo line 35 at r1 (raw file):

#[derive(Debug, Drop, PartialEq, starknet::Event)]
pub struct AddressRemovedFromWhitelist {

Done.


src/payments.cairo line 91 at r1 (raw file):

        tokens: Map<ContractAddress, bool>,
        // Whitelisted addresses.
        whitelisted_addresses: Map<ContractAddress, bool>,

Done.


src/payments.cairo line 292 at r1 (raw file):

        // These functions are used by the contract to control who is allowed to take part in order
        // matching and settlement. Only whitelisted addresses are permitted to trade.

Done.


src/interface.cairo line 21 at r1 (raw file):

    fn is_token_registered(self: @TContractState, token: ContractAddress) -> bool;

    fn whitelist_address(ref self: TContractState, address: ContractAddress);

Done.


src/interface.cairo line 22 at r1 (raw file):

    fn whitelist_address(ref self: TContractState, address: ContractAddress);
    fn remove_from_whitelist(ref self: TContractState, address: ContractAddress);

Done.


src/interface.cairo line 23 at r1 (raw file):

    fn whitelist_address(ref self: TContractState, address: ContractAddress);
    fn remove_from_whitelist(ref self: TContractState, address: ContractAddress);
    fn is_whitelisted(self: @TContractState, address: ContractAddress) -> bool;

Done.


src/errors.cairo line 3 at r1 (raw file):

use starknet::ContractAddress;

pub const ADDRESS_ALREADY_WHITELISTED: felt252 = 'ADDRESS_ALREADY_WHITELISTED';

Done.

Copy link
Collaborator Author

@MohammadNassar1 MohammadNassar1 left a comment

Choose a reason for hiding this comment

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

Done

Reviewable status: 0 of 5 files reviewed, 8 unresolved discussions (waiting on @ImmanuelSegol and @ishay-starkware)

@MohammadNassar1 MohammadNassar1 force-pushed the mohammad/payments/add-address-whitelist branch from fdf9a26 to c0abb9c Compare September 3, 2025 07:11
Copy link
Collaborator

@ishay-starkware ishay-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@ishay-starkware reviewed 2 of 5 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MohammadNassar1)

@MohammadNassar1 MohammadNassar1 force-pushed the mohammad/payments/add-address-whitelist branch from c0abb9c to 35c8127 Compare September 3, 2025 07:26
@MohammadNassar1 MohammadNassar1 enabled auto-merge (squash) September 3, 2025 07:27
@MohammadNassar1 MohammadNassar1 merged commit 03e917b into main Sep 3, 2025
2 of 3 checks passed
@MohammadNassar1 MohammadNassar1 deleted the mohammad/payments/add-address-whitelist branch September 3, 2025 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants