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

Token-locker: comments and review #174

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
45 changes: 41 additions & 4 deletions token-locker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,19 @@ enum StorageKey {
WhitelistAccounts,
}

/// Contract for Bridge Token-Locker which responsible for
/// (1) locking tokens on NEAR side
/// (2) unlocking tokens in case corresponding tokens were burned on Ethereum side
/// Only way to unlock locked tokens is to mint corresponded tokens on Ethereum side and after to burn them.
#[near_bindgen]
#[derive(BorshDeserialize, BorshSerialize, PanicOnDefault)]
pub struct Contract {
/// The account of the prover that we can use to prove.
/// The account of the prover that we can use
/// to prove the authenticity of events generated on Ethereum side.
pub prover_account: AccountId,
/// Ethereum address of the token factory contract, in hex.
pub eth_factory_address: EthAddress,
/// Hashes of the events that were already used.
/// Hashes of the Ethereum `Withdraw` events that were already used for unlock tokens.
pub used_events: UnorderedSet<Vec<u8>>,
/// Mask determining all paused functions
paused: Mask,
Expand All @@ -68,54 +73,80 @@ pub struct Contract {
pub is_whitelist_mode_enabled: bool,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Ethereum Factory also have a whitelist. Are these whitelists consistent with each other? If yes, how is this achieved?

}

/// Interface for Bridge Token-Locker Contract
#[ext_contract(ext_self)]
pub trait ExtContract {
/// The method which called on the last step of the deposit procedure (locking tokens)
#[result_serializer(borsh)]
fn finish_deposit(
&self,
/// Token NEAR address
#[serializer(borsh)] token: AccountId,
/// Amount of tokens to transfer
#[serializer(borsh)] amount: Balance,
/// The Ethereum address of the tokens recipient
#[serializer(borsh)] recipient: EthAddress,
) -> result_types::Lock;

/// The method which called on the last step of the `withdraw` procedure (unlocking tokens)
#[result_serializer(borsh)]
fn finish_withdraw(
&self,
/// This method is called as a callback after `burn` event verification.
/// `verification_success` is a result of verification the proof of the Ethereum `burn` event
#[callback]
#[serializer(borsh)]
verification_success: bool,
/// Account Id of the token on NEAR
#[serializer(borsh)] token: String,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why token is String not an AccountId?

/// Account Id of the token recipient on NEAR
#[serializer(borsh)] new_owner_id: String,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why new_owner_id is a String, not an AccountId? Also, recipient maybe a better name to be consistent with other functions.

/// The amount of the burned tokens
#[serializer(borsh)] amount: Balance,
/// The proof of the Ethereum `burn` event
#[serializer(borsh)] proof: Proof,
) -> Promise;

/// The last step of logging token metadata (the token name, symbols etc).
/// This logging is necessary for transferring information about token to Ethereum
#[result_serializer(borsh)]
fn finish_log_metadata(
&self,
/// The information about token (name, symbols etc)
#[callback] metadata: FungibleTokenMetadata,
/// NEAR account id of the token
token_id: AccountId,
) -> result_types::Metadata;

/// The method which called after checking the token Storage Balance of recipient
/// during the `withdraw` procedure
fn storage_balance_callback(
&self,
/// The result of checking token Storage Balance ot the recipient
#[callback] storage_balance: Option<StorageBalance>,
/// The proof of the Ethereum `burn` event.
#[serializer(borsh)] proof: Proof,
/// The NEAR account id of the token
#[serializer(borsh)] token: String,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why String not AccountId? It is not clear it is NEAR account ID, or token name, or symbols or maybe Eth address...

/// The NEAR account id of the token recipient
#[serializer(borsh)] recipient: AccountId,
/// The amount of burned tokens
#[serializer(borsh)] amount: Balance,
);
}

/// Interface for the ERC20 Bridge Tokens
#[ext_contract(ext_token)]
pub trait ExtToken {
/// Transfer tokens to receiver NEAR account
fn ft_transfer(
&mut self,
receiver_id: AccountId,
amount: U128,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Balance I think is more logical here as a type, but maybe it is standard and can not be changed. Not sure about that.

memo: Option<String>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get what memo mean in this context. But I think it is something standard, and we always use None for that field. So I think it doesn't matter.

) -> PromiseOrValue<U128>;

/// Transfer tokens to receiver NEAR account with specific message
fn ft_transfer_call(
&mut self,
receiver_id: AccountId,
Expand All @@ -124,16 +155,22 @@ pub trait ExtToken {
msg: String,
) -> PromiseOrValue<U128>;

/// Get the information about token (name, symbols etc)
fn ft_metadata(&self) -> FungibleTokenMetadata;

/// Get the storage balance for the specific account
fn storage_balance_of(&mut self, account_id: Option<AccountId>) -> Option<StorageBalance>;
}

#[near_bindgen]
impl Contract {
/// The Bridge Token-Locker initialization.
///
/// # Arguments
///
/// * `prover_account`: NEAR account of the Near Prover contract which check the correctness of the Ethereum Events;
/// * `factory_address`: Ethereum address of the token factory contract, in hex.
#[init]
/// `prover_account`: NEAR account of the Near Prover contract;
/// `factory_address`: Ethereum address of the token factory contract, in hex.
pub fn new(prover_account: AccountId, factory_address: String) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why factory_address is String not EthAddress?

Self {
prover_account,
Expand Down