Skip to content

Commit

Permalink
Fixup of escrow clearing after withdrawing, added tests to check (#246)
Browse files Browse the repository at this point in the history 
* refac: update withdraw function to clear mapping

* refac: setup tests for escrow clearing after withdraw

* refac: fixed all changes requested

* fix: fixup minor errors

* fix: minor changes
  • Loading branch information
@od-hunter
od-hunter authored Oct 14, 2024
1 parent b78a45d commit 455aba1
Show file tree
Hide file tree
Showing 3 changed files with 503 additions and 444 deletions.
152 changes: 74 additions & 78 deletions apps/blockchain/starknet/src/bridge.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ mod bridge {
use starknet::eth_address::EthAddressZeroable;

use openzeppelin::access::ownable::OwnableComponent;
use openzeppelin::access::ownable::interface::{
IOwnableDispatcher, IOwnableDispatcherTrait
};
use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait};

use starklane::interfaces::{IStarklane,
IUpgradeable, IUpgradeableDispatcher, IUpgradeableDispatcherTrait,
IStarklaneCollectionAdmin};
use starklane::interfaces::{
IStarklane, IUpgradeable, IUpgradeableDispatcher, IUpgradeableDispatcherTrait,
IStarklaneCollectionAdmin
};
// events
use starklane::interfaces::{
DepositRequestInitiated, WithdrawRequestCompleted, CollectionDeployedFromL1,
Expand All @@ -30,19 +29,15 @@ mod bridge {
};

use starklane::request::{
Request,
compute_request_header_v1,
compute_request_hash,
collection_type_from_header,
Request, compute_request_header_v1, compute_request_hash, collection_type_from_header,
};
use starklane::token::{
interfaces::{
IERC721Dispatcher, IERC721DispatcherTrait,
IERC721BridgeableDispatcher, IERC721BridgeableDispatcherTrait,
IERC721Dispatcher, IERC721DispatcherTrait, IERC721BridgeableDispatcher,
IERC721BridgeableDispatcherTrait,
},
collection_manager::{
CollectionType,
deploy_erc721_bridgeable, verify_collection_address, erc721_metadata,
CollectionType, deploy_erc721_bridgeable, verify_collection_address, erc721_metadata,
},
};

Expand All @@ -51,7 +46,8 @@ mod bridge {
component!(path: OwnableComponent, storage: ownable, event: OwnableEvent);

#[abi(embed_v0)]
impl OwnableTwoStepMixinImpl = OwnableComponent::OwnableTwoStepMixinImpl<ContractState>;
impl OwnableTwoStepMixinImpl =
OwnableComponent::OwnableTwoStepMixinImpl<ContractState>;

impl OwnableInternalImpl = OwnableComponent::InternalImpl<ContractState>;

Expand All @@ -68,17 +64,13 @@ mod bridge {
// Registry of escrowed token for collections.
// <(collection_l2_address, token_id), original_depositor_l2_address>
escrow: LegacyMap::<(ContractAddress, u256), ContractAddress>,

// White list enabled flag
white_list_enabled: bool,

// Registry of whitelisted collections
white_listed_list: LegacyMap::<ContractAddress, (bool, ContractAddress)>,
white_listed_head: ContractAddress,

// Bridge enabled flag
enabled: bool,

#[substorage(v0)]
ownable: OwnableComponent::Storage,
}
Expand Down Expand Up @@ -127,14 +119,9 @@ mod bridge {
/// to be more flexible? And the first felt25 being the header
/// defines how the deserialization takes place?
#[l1_handler]
fn withdraw_auto_from_l1(
ref self: ContractState,
from_address: felt252,
req: Request
) {
fn withdraw_auto_from_l1(ref self: ContractState, from_address: felt252, req: Request) {
ensure_is_enabled(@self);
assert(self.bridge_l1_address.read().into() == from_address,
'Invalid L1 msg sender');
assert(self.bridge_l1_address.read().into() == from_address, 'Invalid L1 msg sender');

// TODO: recompute HASH to ensure data are not altered.
// TODO: Validate all fields the request (cf. FSM).
Expand All @@ -159,26 +146,32 @@ mod bridge {

if is_escrowed {
IERC721Dispatcher { contract_address: collection_l2 }
.transfer_from(from, to, token_id);
.transfer_from(from, to, token_id);

let null_value = starknet::contract_address_const::<0>();
self.escrow.write((collection_l2, token_id), null_value);
} else {
if (req.uris.len() != 0) {
let token_uri = req.uris[i];
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge_uri(to, token_id, token_uri.clone());
.mint_from_bridge_uri(to, token_id, token_uri.clone());
} else {
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge(to, token_id);
.mint_from_bridge(to, token_id);
}
}

i += 1;
};

self.emit(WithdrawRequestCompleted {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
});
self
.emit(
WithdrawRequestCompleted {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
}
);
}

#[abi(embed_v0)]
Expand All @@ -188,10 +181,12 @@ mod bridge {

match starknet::replace_class_syscall(class_hash) {
Result::Ok(_) => {
self.emit(ReplacedClassHash {
contract: starknet::get_contract_address(),
class: class_hash,
})
self
.emit(
ReplacedClassHash {
contract: starknet::get_contract_address(), class: class_hash,
}
)
},
Result::Err(revert_reason) => panic(revert_reason),
};
Expand All @@ -200,7 +195,6 @@ mod bridge {

#[abi(embed_v0)]
impl BridgeImpl of IStarklane<ContractState> {

fn get_l1_collection_address(self: @ContractState, address: ContractAddress) -> EthAddress {
self.l2_to_l1_addresses.read(address)
}
Expand Down Expand Up @@ -237,8 +231,10 @@ mod bridge {
/// * `collection_l2` - Address of the collection on L2.
/// * `owner_l1` - Address of the owner on L1.
/// * `tokens_ids` - Tokens to be bridged on L1.
/// * `use_withdraw_auto` - Tokens are automatically withdrawn on L1 using Starklane indexer.
/// * `use_deposit_burn_auto` - Tokens are automatically burnt on L2 once transferred using Starklane indexer.
/// * `use_withdraw_auto` - Tokens are automatically withdrawn on L1 using Starklane
/// indexer.
/// * `use_deposit_burn_auto` - Tokens are automatically burnt on L2 once transferred using
/// Starklane indexer.
///
/// TODO: add new_owners, values and uris.
/// TODO: better to use a struct than too much arguments? (DepositParams/DepositInputs/???)
Expand Down Expand Up @@ -295,17 +291,17 @@ mod bridge {
let mut buf: Array<felt252> = array![];
req.serialize(ref buf);

starknet::send_message_to_l1_syscall(
self.bridge_l1_address.read().into(),
buf.span(),
)
starknet::send_message_to_l1_syscall(self.bridge_l1_address.read().into(), buf.span(),)
.unwrap_syscall();

self.emit(DepositRequestInitiated {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
});

self
.emit(
DepositRequestInitiated {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
}
);
}

fn enable_white_list(ref self: ContractState, enable: bool) {
Expand All @@ -318,13 +314,12 @@ mod bridge {
self.white_list_enabled.read()
}

fn white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) {
fn white_list_collection(
ref self: ContractState, collection: ContractAddress, enabled: bool
) {
ensure_is_admin(@self);
_white_list_collection(ref self, collection, enabled);
self.emit(CollectionWhiteListUpdated {
collection,
enabled,
});
self.emit(CollectionWhiteListUpdated { collection, enabled, });
}

fn is_white_listed(self: @ContractState, collection: ContractAddress) -> bool {
Expand Down Expand Up @@ -352,16 +347,16 @@ mod bridge {
fn enable(ref self: ContractState, enable: bool) {
ensure_is_admin(@self);
self.enabled.write(enable);
self.emit(BridgeEnabled {
enable: enable
});
self.emit(BridgeEnabled { enable: enable });
}

fn is_enabled(self: @ContractState) -> bool {
self.enabled.read()
}

fn set_l1_l2_collection_mapping(ref self: ContractState, collection_l1: EthAddress, collection_l2: ContractAddress) {
fn set_l1_l2_collection_mapping(
ref self: ContractState, collection_l1: EthAddress, collection_l2: ContractAddress
) {
ensure_is_admin(@self);
self.l1_to_l2_addresses.write(collection_l1, collection_l2);
self.l2_to_l1_addresses.write(collection_l2, collection_l1);
Expand All @@ -371,17 +366,19 @@ mod bridge {

#[abi(embed_v0)]
impl BridgeCollectionAdminImpl of IStarklaneCollectionAdmin<ContractState> {
fn collection_upgrade(ref self: ContractState, collection: ContractAddress, class_hash: ClassHash) {
fn collection_upgrade(
ref self: ContractState, collection: ContractAddress, class_hash: ClassHash
) {
ensure_is_admin(@self);
IUpgradeableDispatcher { contract_address: collection }
.upgrade(class_hash);
IUpgradeableDispatcher { contract_address: collection }.upgrade(class_hash);
}

// transfer owner of the given collection to the given address
fn collection_transfer_ownership(ref self: ContractState, collection: ContractAddress, new_owner: ContractAddress) {
fn collection_transfer_ownership(
ref self: ContractState, collection: ContractAddress, new_owner: ContractAddress
) {
ensure_is_admin(@self);
IOwnableDispatcher { contract_address: collection }
.transfer_ownership(new_owner);
IOwnableDispatcher { contract_address: collection }.transfer_ownership(new_owner);
}
}

Expand Down Expand Up @@ -431,7 +428,6 @@ mod bridge {
///
/// * `req` - Request for bridging assets.
fn ensure_erc721_deployment(ref self: ContractState, req: @Request) -> ContractAddress {

let l1_req: EthAddress = *req.collection_l1;
let l2_req: ContractAddress = *req.collection_l2;

Expand Down Expand Up @@ -476,21 +472,21 @@ mod bridge {
let (already_white_listed, _) = self.white_listed_list.read(l2_addr_from_deploy);
if already_white_listed != true {
_white_list_collection(ref self, l2_addr_from_deploy, true);
self.emit(CollectionWhiteListUpdated {
collection: l2_addr_from_deploy,
enabled: true,
});
self
.emit(
CollectionWhiteListUpdated { collection: l2_addr_from_deploy, enabled: true, }
);
}
l2_addr_from_deploy
}

fn _is_white_listed(self: @ContractState, collection: ContractAddress) -> bool {
let enabled = self.white_list_enabled.read();
if (enabled) {
let (ret, _) = self.white_listed_list.read(collection);
return ret;
}
true
let enabled = self.white_list_enabled.read();
if (enabled) {
let (ret, _) = self.white_listed_list.read(collection);
return ret;
}
true
}

fn _white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) {
Expand All @@ -517,7 +513,7 @@ mod bridge {
prev = next;
};
self.white_listed_list.write(prev, (true, collection));
} else {
} else {
// change head
if prev == collection {
let (_, next) = self.white_listed_list.read(prev);
Expand Down
9 changes: 6 additions & 3 deletions apps/blockchain/starknet/src/interfaces.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use starklane::request::Request;

#[starknet::interface]
trait IStarklane<T> {

fn deposit_tokens(
ref self: T,
salt: felt252,
Expand Down Expand Up @@ -33,7 +32,9 @@ trait IStarklane<T> {
fn enable(ref self: T, enable: bool);
fn is_enabled(self: @T) -> bool;

fn set_l1_l2_collection_mapping(ref self: T , collection_l1: EthAddress, collection_l2: ContractAddress);
fn set_l1_l2_collection_mapping(
ref self: T, collection_l1: EthAddress, collection_l2: ContractAddress
);
}

/// Upgradeable contract.
Expand All @@ -48,7 +49,9 @@ trait IStarklaneCollectionAdmin<T> {
fn collection_upgrade(ref self: T, collection: ContractAddress, class_hash: ClassHash);

// transfer owner of the given collection to the given address
fn collection_transfer_ownership(ref self: T, collection: ContractAddress, new_owner: ContractAddress);
fn collection_transfer_ownership(
ref self: T, collection: ContractAddress, new_owner: ContractAddress
);
}

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

0 comments on commit 455aba1

Please sign in to comment.