From 27a7e04e86f735c0f57881b4e17b6067715d0809 Mon Sep 17 00:00:00 2001 From: Doris Benda Date: Thu, 30 Nov 2023 17:56:01 +0300 Subject: [PATCH] Address comments --- concordium-cis2/src/lib.rs | 22 +-- .../sponsored-tx-enabled-auction/Cargo.toml | 2 +- .../sponsored-tx-enabled-auction/src/lib.rs | 126 ++++++++---------- .../tests/tests.rs | 2 +- 4 files changed, 62 insertions(+), 90 deletions(-) diff --git a/concordium-cis2/src/lib.rs b/concordium-cis2/src/lib.rs index b30123dc..cdda68f3 100644 --- a/concordium-cis2/src/lib.rs +++ b/concordium-cis2/src/lib.rs @@ -1267,8 +1267,6 @@ pub struct OnReceivingCis2DataParams { } /// Deserial trait for OnReceivingCis2DataParams. -/// The specific type D is constructed from the general `AdditionalData` type -/// first before the `OnReceivingCis2DataParams` object is deserialized. impl Deserial for OnReceivingCis2DataParams { fn deserial(source: &mut R) -> ParseResult { let params: OnReceivingCis2Params = source.get()?; @@ -1283,21 +1281,13 @@ impl Deserial for OnReceivingCis2DataPara } /// Serial trait for OnReceivingCis2DataParams. -/// The specific type D is converted into the general `AdditionalData` type -/// first before the `OnReceivingCis2Params` object is serialized. -impl Serial - for OnReceivingCis2DataParams -{ +impl Serial for OnReceivingCis2DataParams { fn serial(&self, out: &mut W) -> Result<(), W::Err> { - let additional_data = AdditionalData::from(to_bytes(&self.data)); - let object: OnReceivingCis2Params = OnReceivingCis2Params { - token_id: self.token_id.clone(), - amount: self.amount.clone(), - from: self.from, - data: additional_data, - }; - - object.serial(out)?; + self.token_id.serial(out)?; + self.amount.serial(out)?; + self.from.serial(out)?; + (to_bytes(&self.data).len() as u16).serial(out)?; + out.write_all(&to_bytes(&self.data))?; Ok(()) } } diff --git a/examples/sponsored-tx-enabled-auction/Cargo.toml b/examples/sponsored-tx-enabled-auction/Cargo.toml index 527b1bcf..b5740d7f 100644 --- a/examples/sponsored-tx-enabled-auction/Cargo.toml +++ b/examples/sponsored-tx-enabled-auction/Cargo.toml @@ -17,7 +17,7 @@ concordium-cis2 = {path = "../../concordium-cis2", default-features = false} [dev-dependencies] concordium-smart-contract-testing = { path = "../../contract-testing" } cis2-multi = { path = "../cis2-multi/" } -rand = "0.7.0" +rand = "0.7" [lib] crate-type=["cdylib", "rlib"] diff --git a/examples/sponsored-tx-enabled-auction/src/lib.rs b/examples/sponsored-tx-enabled-auction/src/lib.rs index 81c7afc0..89ecf54d 100644 --- a/examples/sponsored-tx-enabled-auction/src/lib.rs +++ b/examples/sponsored-tx-enabled-auction/src/lib.rs @@ -36,11 +36,6 @@ //! auction. The creator of that auction receives the highest bid when the //! auction is finalized and the item is marked as sold to the highest bidder. //! This can be done only once. -//! -//! Terminology: `Accounts` are derived from a public/private key pair. -//! `Contract` instances are created by deploying a smart contract -//! module and initializing it. - #![cfg_attr(not(feature = "std"), no_std)] use concordium_cis2::{Cis2Client, *}; @@ -183,22 +178,37 @@ pub enum Error { /// Raised when payment is attempted with a different `token_id` than /// specified for an item. WrongTokenID, //-11 + /// Raised when the invocation of the cis2 token contract fails. InvokeContractError, //-12 - ParseResult, //-13 + /// Raised when the parsing of the result from the cis2 token contract + /// fails. + ParseResult, //-13 + /// Raised when the response of the cis2 token contract is invalid. InvalidResponse, //-14 + /// Raised when the amount of cis2 tokens that was to be transferred is not + /// available to the sender. AmountTooLarge, //-15 + /// Raised when the owner account of the cis 2 token contract that is being + /// invoked does not exist. This variant should in principle not happen, + /// but is here for completeness. MissingAccount, //-16 + /// Raised when the cis2 token contract that is to be invoked does not + /// exist. MissingContract, //-17 + /// Raised when the cis2 token contract to be invoked exists, but the entry + /// point that was named does not. MissingEntrypoint, //-18 + // Raised when the sending of a message to the V0 contract failed. MessageFailed, //-19 - LogicReject, //-20 - Trap, //-21 - TransferFailed, //-22 + // Raised when the cis2 token contract called rejected with the given reason. + LogicReject, //-20 + // Raised when the cis2 token contract execution triggered a runtime error. + Trap, //-21 } pub type ContractResult = Result; -/// Mapping Cis2ClientError to Error +/// Mapping CallContractError to Error impl From> for Error { fn from(e: CallContractError) -> Self { match e { @@ -266,9 +276,9 @@ fn add_item(ctx: &ReceiveContext, host: &mut Host) -> ContractResult<()> // Ensure start < end. ensure!(item.start < item.end, Error::StartEndTimeError); - let slot_time = ctx.metadata().slot_time(); + let block_time = ctx.metadata().block_time(); // Ensure the auction can run. - ensure!(slot_time <= item.end, Error::EndTimeError); + ensure!(block_time <= item.end, Error::EndTimeError); // Assign an index to the item/auction. let counter = host.state_mut().counter; @@ -314,6 +324,8 @@ fn auction_bid(ctx: &ReceiveContext, host: &mut Host) -> ContractResult<( bail!(Error::NotTokenContract); } + let contract = host.state().cis2_contract; + // Getting input parameters. let params: OnReceivingCis2DataParams< ContractTokenId, @@ -336,9 +348,9 @@ fn auction_bid(ctx: &ReceiveContext, host: &mut Host) -> ContractResult<( // Ensure the auction has not been finalized yet. ensure_eq!(item.auction_state, AuctionState::NotSoldYet, Error::AuctionAlreadyFinalized); - let slot_time = ctx.metadata().slot_time(); + let block_time = ctx.metadata().block_time(); // Ensure the auction has not ended yet. - ensure!(slot_time <= item.end, Error::BidTooLate); + ensure!(block_time <= item.end, Error::BidTooLate); // Ensure that the new bid exceeds the highest bid so far. let old_highest_bid = item.highest_bid; @@ -348,6 +360,12 @@ fn auction_bid(ctx: &ReceiveContext, host: &mut Host) -> ContractResult<( item.highest_bid = params.amount; if let Some(account_address) = item.highest_bidder.replace(bidder_address) { + let client = Cis2Client::new(contract); + + let read_item = item.clone(); + + drop(item); + // Refunding old highest bidder. // This transfer (given enough NRG of course) always succeeds because // the `account_address` exists since it was recorded when it @@ -356,24 +374,13 @@ fn auction_bid(ctx: &ReceiveContext, host: &mut Host) -> ContractResult<( // Please consider using a pull-over-push pattern when expanding this // smart contract to allow smart contract instances to // participate in the auction as well. https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/ - let parameter = TransferParameter { - 0: vec![Transfer { - token_id: item.token_id, - amount: old_highest_bid, - from: concordium_std::Address::Contract(ctx.self_address()), - to: concordium_cis2::Receiver::Account(account_address), - data: AdditionalData::empty(), - }], - }; - - drop(item); - - host.invoke_contract( - &host.state().cis2_contract.clone(), - ¶meter, - EntrypointName::new_unchecked("transfer"), - Amount::zero(), - )?; + client.transfer::(host, Transfer { + amount: old_highest_bid, + from: concordium_std::Address::Contract(ctx.self_address()), + to: concordium_cis2::Receiver::Account(account_address), + token_id: read_item.token_id, + data: AdditionalData::empty(), + })?; } Ok(()) @@ -401,15 +408,21 @@ fn auction_finalize(ctx: &ReceiveContext, host: &mut Host) -> ContractRes // Ensure the auction has not been finalized yet. ensure_eq!(item.auction_state, AuctionState::NotSoldYet, Error::AuctionAlreadyFinalized); - let slot_time = ctx.metadata().slot_time(); + let block_time = ctx.metadata().block_time(); // Ensure the auction has ended already. - ensure!(slot_time > item.end, Error::AuctionStillActive); + ensure!(block_time > item.end, Error::AuctionStillActive); if let Some(account_address) = item.highest_bidder { // Marking the highest bidder (the last accepted bidder) as winner of the // auction. item.auction_state = AuctionState::Sold(account_address); + let client = Cis2Client::new(contract); + + let read_item = item.clone(); + + drop(item); + // Sending the highest bid in tokens to the creator of the auction. // This transfer (given enough NRG of course) always succeeds because // the `creator` exists since it was recorded when it @@ -418,44 +431,13 @@ fn auction_finalize(ctx: &ReceiveContext, host: &mut Host) -> ContractRes // Please consider using a pull-over-push pattern when expanding this // smart contract to allow smart contract instances to // participate in the auction as well. https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/ - // let parameter = TransferParameter { - // 0: vec![Transfer { - // token_id: item.token_id, - // amount: item.highest_bid, - // from: concordium_std::Address::Contract(ctx.self_address()), - // to: concordium_cis2::Receiver::Account(item.creator), - // data: AdditionalData::empty(), - // }], - // }; - - // drop(item); - - let client = Cis2Client::new(contract); - - let read_item = item.clone(); - - drop(item); - - let success = client.transfer::( - host, - Transfer { - amount: read_item.highest_bid, - from: concordium_std::Address::Contract(ctx.self_address()), - to: concordium_cis2::Receiver::Account(read_item.creator), - token_id: read_item.token_id, - data: AdditionalData::empty(), - }, - )?; - - // Ensure the transfer was successful ??? Shouldn't this return `true` - ensure!(!success, Error::TransferFailed); - - // host.invoke_contract( - // &host.state().cis2_contract.clone(), - // ¶meter, - // EntrypointName::new_unchecked("transfer"), - // Amount::zero(), - // )?; + client.transfer::(host, Transfer { + amount: read_item.highest_bid, + from: concordium_std::Address::Contract(ctx.self_address()), + to: concordium_cis2::Receiver::Account(read_item.creator), + token_id: read_item.token_id, + data: AdditionalData::empty(), + })?; } Ok(()) diff --git a/examples/sponsored-tx-enabled-auction/tests/tests.rs b/examples/sponsored-tx-enabled-auction/tests/tests.rs index 20a5475d..2c28bbe8 100644 --- a/examples/sponsored-tx-enabled-auction/tests/tests.rs +++ b/examples/sponsored-tx-enabled-auction/tests/tests.rs @@ -658,7 +658,7 @@ fn view_item_state(chain: &Chain, auction_contract_address: ContractAddress) -> invoke.parse_return_value().expect("ViewItemState return value") } -/// Get the `TOKEN_1` balances for Alice and the auction contract. +/// Get the `TokenIdU8(1)` balances for Alice and the auction contract. fn get_balances( chain: &Chain, auction_contract_address: ContractAddress,