Skip to content

Commit

Permalink
feat(broker fees): remove obsolete public methods from contract (#410)
Browse files Browse the repository at this point in the history
## Description
The following public methods have been removed from `orderbook`
contract:
- whitelist_broker
- unwhitelist_broker
- create_order
- cancel_order
- fulfill_order

`set_broker_fees` have been updated to use `caller` instead of
`broker_address` argument.

Fees denominator are limited to 10000 to prevent overflow in the
executor contract.

<!--
Please do not leave this blank.
Describe the changes in this PR. What does it [add/remove/fix/replace]?

For crafting a good description, consider using ChatGPT to help
articulate your changes.
-->

## What type of PR is this? (check all applicable)

- [x] 🍕 Feature (`feat:`)
- [x] 🐛 Bug Fix (`fix:`)
- [ ] 📝 Documentation Update (`docs:`)
- [ ] 🎨 Style (`style:`)
- [ ] 🧑‍💻 Code Refactor (`refactor:`)
- [ ] 🔥 Performance Improvements (`perf:`)
- [x] ✅ Test (`test:`)
- [ ] 🤖 Build (`build:`)
- [ ] 🔁 CI (`ci:`)
- [ ] 📦 Chore (`chore:`)
- [ ] ⏩ Revert (`revert:`)
- [ ] 🚀 Breaking Changes (`BREAKING CHANGE:`)

## Related Tickets & Documents

<!--
Please use this format to link related issues: Fixes #<issue_number>
More info:
https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->

## Added tests?

- [ ] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help

## Added to documentation?

- [ ] 📜 README.md
- [ ] 📓 Documentation
- [ ] 🙅 no documentation needed

## [optional] Are there any post-deployment tasks we need to perform?

<!-- Describe any additional tasks, if any, and provide steps. -->

## [optional] What gif best describes this PR or how it makes you feel?

<!-- Share a fun gif related to your PR! -->

### PR Title and Description Guidelines:

- Ensure your PR title follows semantic versioning standards. This helps
automate releases and changelogs.
- Use types like `feat:`, `fix:`, `chore:`, `BREAKING CHANGE:` etc. in
your PR title.
- Your PR title will be used as a commit message when merging. Make sure
it adheres to [Conventional Commits
standards](https://www.conventionalcommits.org/).

## Closing Issues

<!--
Use keywords to close related issues. This ensures that the associated
issues will automatically close when the PR is merged.

- `Fixes #123` will close issue 123 when the PR is merged.
- `Closes #123` will also close issue 123 when the PR is merged.
- `Resolves #123` will also close issue 123 when the PR is merged.

You can also use multiple keywords in one comment:
- `Fixes #123, Resolves #456`

More info:
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
-->

---------

Co-authored-by: christophe <[email protected]>
Co-authored-by: kwiss <[email protected]>
  • Loading branch information
3 people authored Jul 17, 2024
1 parent 91de867 commit 8702979
Show file tree
Hide file tree
Showing 62 changed files with 2,232 additions and 2,784 deletions.
1 change: 1 addition & 0 deletions addresses.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"orderbook_address":"0x2281b4a351f12ee30f940ac2dd1e48f03b52c76c8f14169322b0f05d6a7fc32","sn_executor_address":"0x2b3a70ba9f135220d06433c4ec8a313ef8363f53e75371c222ff3895bcce7c6"}
1 change: 0 additions & 1 deletion contracts/ark_common/src/lib.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ mod protocol {
mod order_types;
mod order_v1;
mod order_database;
mod broker;
}

mod crypto {
Expand Down
70 changes: 0 additions & 70 deletions contracts/ark_common/src/protocol/broker.cairo

This file was deleted.

4 changes: 0 additions & 4 deletions contracts/ark_common/src/protocol/order_types.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ enum OrderValidationError {
AdditionalDataTooLong,
InvalidContent,
InvalidSalt,
InvalidBroker,
}

impl OrderValidationErrorIntoFelt252 of Into<OrderValidationError, felt252> {
Expand All @@ -61,7 +60,6 @@ impl OrderValidationErrorIntoFelt252 of Into<OrderValidationError, felt252> {
OrderValidationError::AdditionalDataTooLong => 'ADDITIONAL_DATA_TOO_LONG',
OrderValidationError::InvalidContent => 'INVALID_CONTENT',
OrderValidationError::InvalidSalt => 'INVALID_SALT',
OrderValidationError::InvalidBroker => 'INVALID_BROKER',
}
}
}
Expand All @@ -78,8 +76,6 @@ impl Felt252TryIntoOrderValidationError of TryInto<felt252, OrderValidationError
Option::Some(OrderValidationError::InvalidContent)
} else if self == 'INVALID_SALT' {
Option::Some(OrderValidationError::InvalidSalt)
} else if self == 'INVALID_BROKER' {
Option::Some(OrderValidationError::InvalidBroker)
} else {
Option::None
}
Expand Down
11 changes: 0 additions & 11 deletions contracts/ark_common/src/protocol/order_v1.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use core::traits::Into;
use core::traits::TryInto;
use core::option::OptionTrait;

use ark_common::protocol::broker::{broker_whitelist_read};

//! Order v1 supported by the Orderbook.
//!
use starknet::ContractAddress;
Expand All @@ -14,8 +12,6 @@ use ark_common::protocol::order_types::FulfillInfo;
use poseidon::poseidon_hash_span;
use starknet::SyscallResultTrait;

/// Must remain equal to 0 for now.
const ADDRESS_DOMAIN: u32 = 0;
const ORDER_VERSION_V1: felt252 = 'v1';
// Auction -> end_amount (reserve price) > start_amount (starting price).
// Auction -> ERC721_ERC20.
Expand Down Expand Up @@ -96,13 +92,6 @@ impl OrderTraitOrderV1 of OrderTrait<OrderV1> {
return Result::Err(OrderValidationError::InvalidContent);
}

// check if the broker is whitelisted.
let whitelisted = broker_whitelist_read(*self.broker_id);

if whitelisted == false {
return Result::Err(OrderValidationError::InvalidBroker);
}

Result::Ok(())
}

Expand Down
191 changes: 0 additions & 191 deletions contracts/ark_orderbook/src/orderbook.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,6 @@ use starknet::ContractAddress;
/// Orderbook trait to define operations on orderbooks.
#[starknet::interface]
trait Orderbook<T> {
/// Whitelists a broker.
///
/// # Arguments
///
/// * `broker_id` - ID of the broker.
fn whitelist_broker(ref self: T, broker_id: ContractAddress);

/// Remove a broker from the whitelist.
///
/// # Arguments
///
/// * `broker_id` - ID of the broker.
fn unwhitelist_broker(ref self: T, broker_id: ContractAddress);

/// Submits and places an order to the orderbook if the order is valid.
///
/// # Arguments
///
/// * `order` - The order to be placed.
/// * `sign_info` - The signing info of the `order`.
fn create_order(ref self: T, order: OrderV1, signer: Signer);

/// Cancels an existing order in the orderbook.
///
/// # Arguments
///
/// * `cancel_info` - information about the order to be cancelled.
/// * `sign_info` - The signing information associated with the order cancellation.
fn cancel_order(ref self: T, cancel_info: CancelInfo, signer: Signer);

/// Fulfils an existing order in the orderbook.
///
/// # Arguments
///
/// * `order_hash` - The order to be fulfil.
/// * `sign_info` - The signing information associated with the order fulfillment.
fn fulfill_order(ref self: T, fulfill_info: FulfillInfo, signer: Signer);

/// Retrieves the type of an order using its hash.
///
/// # Arguments
Expand All @@ -75,12 +37,6 @@ trait Orderbook<T> {
/// * `order_hash` - The order hash of order.
fn get_order(self: @T, order_hash: felt252) -> OrderV1;

/// Retrieves the order signer using its hash.
///
/// # Arguments
/// * `order_hash` - The order hash of order.
fn get_order_signer(self: @T, order_hash: felt252) -> felt252;

/// Retrieves the order hash using its token hash.
///
/// # Arguments
Expand Down Expand Up @@ -153,8 +109,6 @@ mod orderbook {
order_read, order_status_read, order_write, order_status_write, order_type_read
};

use ark_common::protocol::broker::{broker_whitelist_write};

const EXTENSION_TIME_IN_SECONDS: u64 = 600;
const AUCTION_ACCEPTING_TIME_SECS: u64 = 172800;
/// Storage struct for the Orderbook contract.
Expand All @@ -176,8 +130,6 @@ mod orderbook {
auctions: LegacyMap<felt252, (felt252, u64, u256)>,
/// Mapping of auction offer order_hash to auction listing order_hash.
auction_offers: LegacyMap<felt252, felt252>,
/// Mapping of order_hash to order_signer public key.
order_signers: LegacyMap<felt252, felt252>,
/// The address of the StarkNet executor contract.
starknet_executor_address: ContractAddress,
}
Expand Down Expand Up @@ -377,16 +329,6 @@ mod orderbook {
order.unwrap()
}

/// Retrieves the order signer using its hash.
/// # View
fn get_order_signer(self: @ContractState, order_hash: felt252) -> felt252 {
let order_signer = self.order_signers.read(order_hash);
if (order_signer.is_zero()) {
panic_with_felt252(orderbook_errors::ORDER_NOT_FOUND);
}
order_signer
}

/// Retrieves the order hash using its token hash.
/// # View
fn get_order_hash(self: @ContractState, token_hash: felt252) -> felt252 {
Expand All @@ -396,139 +338,6 @@ mod orderbook {
}
order_hash
}

/// Whitelists a broker.
fn whitelist_broker(ref self: ContractState, broker_id: ContractAddress) {
assert(starknet::get_caller_address() == self.admin.read(), 'Unauthorized update');
broker_whitelist_write(broker_id, 1);
}

/// Remove a broker from whitelist.
fn unwhitelist_broker(ref self: ContractState, broker_id: ContractAddress) {
assert(starknet::get_caller_address() == self.admin.read(), 'Unauthorized update');
broker_whitelist_write(broker_id, 0);
}

/// Submits and places an order to the orderbook if the order is valid.
fn create_order(ref self: ContractState, order: OrderV1, signer: Signer) {
let order_hash = order.compute_order_hash();
let order_sign = OrderSign { hash: order_hash };
let order_sign_hash = order_sign
.compute_hash_from(from: order.offerer, chain_id: self.chain_id.read());
let user_pubkey = SignerValidator::verify(order_sign_hash, signer);
let block_ts = starknet::get_block_timestamp();
let validation = order.validate_common_data(block_ts);
if validation.is_err() {
panic_with_felt252(validation.unwrap_err().into());
}
let order_type = order
.validate_order_type()
.expect(orderbook_errors::ORDER_INVALID_DATA);
let order_hash = order.compute_order_hash();
match order_type {
OrderType::Listing => {
assert(
order_status_read(order_hash).is_none(),
orderbook_errors::ORDER_ALREADY_EXISTS
);
let _ = self._create_listing_order(order, order_type, order_hash);
},
OrderType::Auction => {
assert(
order_status_read(order_hash).is_none(),
orderbook_errors::ORDER_ALREADY_EXISTS
);
self._create_auction(order, order_type, order_hash);
},
OrderType::Offer => { self._create_offer(order, order_type, order_hash); },
OrderType::CollectionOffer => {
self._create_collection_offer(order, order_type, order_hash);
},
};
self.order_signers.write(order_hash, user_pubkey);
}

fn cancel_order(ref self: ContractState, cancel_info: CancelInfo, signer: Signer) {
let order_hash = cancel_info.order_hash;
let original_signer_public_key = self.order_signers.read(order_hash);
let mut canceller_signer = signer.clone();
canceller_signer.set_public_key(original_signer_public_key);
let cancel_info_hash = serialized_hash(cancel_info);
let order_sign = OrderSign { hash: cancel_info_hash };
let order_sign_hash = order_sign
.compute_hash_from(from: cancel_info.canceller, chain_id: self.chain_id.read());

SignerValidator::verify(order_sign_hash, canceller_signer);
let order_option = order_read::<OrderV1>(order_hash);
assert(order_option.is_some(), orderbook_errors::ORDER_NOT_FOUND);
let order = order_option.unwrap();
assert(order.offerer == cancel_info.canceller, 'not the same offerrer');
match order_status_read(order_hash) {
Option::Some(s) => s,
Option::None => panic_with_felt252(orderbook_errors::ORDER_NOT_FOUND),
};
let block_ts = starknet::get_block_timestamp();
match order_type_read(order_hash) {
Option::Some(order_type) => {
if order_type == OrderType::Auction {
let auction_token_hash = order.compute_token_hash();
let (_, auction_end_date, _) = self.auctions.read(auction_token_hash);
assert(
block_ts <= auction_end_date, orderbook_errors::ORDER_AUCTION_IS_EXPIRED
);
self.auctions.write(auction_token_hash, (0, 0, 0));
} else {
assert(block_ts < order.end_date, orderbook_errors::ORDER_IS_EXPIRED);
if order_type == OrderType::Listing {
self.token_listings.write(order.compute_token_hash(), 0);
}
}
},
Option::None => panic_with_felt252(orderbook_errors::ORDER_NOT_FOUND),
};

// Cancel order
order_status_write(order_hash, OrderStatus::CancelledUser);
self.emit(OrderCancelled { order_hash, reason: OrderStatus::CancelledUser.into() });
}

fn fulfill_order(ref self: ContractState, fulfill_info: FulfillInfo, signer: Signer) {
let fulfill_hash = serialized_hash(fulfill_info);
let fulfill_sign = OrderSign { hash: fulfill_hash };
let fulfill_sign_hash = fulfill_sign
.compute_hash_from(from: fulfill_info.fulfiller, chain_id: self.chain_id.read());

SignerValidator::verify(fulfill_sign_hash, signer);

let order_hash = fulfill_info.order_hash;
let order: OrderV1 = match order_read(order_hash) {
Option::Some(o) => o,
Option::None => panic_with_felt252(orderbook_errors::ORDER_NOT_FOUND),
};
let status = match order_status_read(order_hash) {
Option::Some(s) => s,
Option::None => panic_with_felt252(orderbook_errors::ORDER_NOT_FOUND),
};
assert(status == OrderStatus::Open, orderbook_errors::ORDER_NOT_FULFILLABLE);
let order_type = match order_type_read(order_hash) {
Option::Some(s) => s,
Option::None => panic_with_felt252(orderbook_errors::ORDER_NOT_FOUND),
};
match order_type {
OrderType::Listing => { self._fulfill_listing_order(fulfill_info, order); },
OrderType::Auction => {
let original_signer_public_key = self
.order_signers
.read(fulfill_info.order_hash);
let mut origin_signer = signer.clone();
origin_signer.set_public_key(original_signer_public_key);
SignerValidator::verify(fulfill_sign_hash, origin_signer);
self._fulfill_auction_order(fulfill_info, order)
},
OrderType::Offer => { self._fulfill_offer(fulfill_info, order); },
OrderType::CollectionOffer => { self._fulfill_offer(fulfill_info, order); }
}
}
}

// *************************************************************************
Expand Down
Loading

0 comments on commit 8702979

Please sign in to comment.