Feat/make transfer #14
Conversation
tests/lib.cairo
Outdated
| } | ||
|
|
||
| #[test] | ||
| fn test_check_owner_and_operator_empty_array() { |
There was a problem hiding this comment.
we already check for this
There was a problem hiding this comment.
I will remove it
tests/lib.cairo
Outdated
| } | ||
|
|
||
| #[test] | ||
| fn test_check_owner_and_operator_zero_balance() { |
There was a problem hiding this comment.
This looks more like a test for a token that doesn't exist than a zero balance
There was a problem hiding this comment.
taking a look at it
tests/lib.cairo
Outdated
| fn test_check_owner_and_operator_empty_transfers() { | ||
| let (tokenized_bond, _minter) = setup_contract_with_minter(); | ||
| let transfers = array![]; | ||
| start_cheat_caller_address(tokenized_bond.contract_address, NOT_MINTER()); |
There was a problem hiding this comment.
Why are we calling this contract with the not_minter address?
There was a problem hiding this comment.
This is to check if check_owner_and_operator functions returns false
There was a problem hiding this comment.
we don't really need to cheat the caller at all here
There was a problem hiding this comment.
yeah, I just deleted that. Thanks for pointing it out.
tests/lib.cairo
Outdated
| fn test_check_owner_and_operator_zero_balance() { | ||
| let (tokenized_bond, minter) = setup_contract_with_minter(); | ||
| let receiver1 = setup_receiver(); | ||
| let other_account = setup_receiver(); |
There was a problem hiding this comment.
Other account is a very vague name for a variable and doesn't follow the naming conventions of this function or the others
There was a problem hiding this comment.
I will change it to receiver2.
tests/lib.cairo
Outdated
| let other_account = setup_receiver(); | ||
|
|
||
| start_cheat_caller_address(tokenized_bond.contract_address, OWNER()); | ||
| tokenized_bond.add_minter(other_account); |
There was a problem hiding this comment.
why are we making this other account a minter
There was a problem hiding this comment.
This is for testing transfers from minter who has zero balance.
tests/lib.cairo
Outdated
|
|
||
| start_cheat_caller_address(tokenized_bond.contract_address, other_account); | ||
|
|
||
| let zero_balance_destination = array![ |
There was a problem hiding this comment.
Why are we testing for a zero balance at the destination?
There was a problem hiding this comment.
The test is to verify that an account cannot transfer tokens if they don't have any tokens to transfer, even if they are a valid minter.
tests/lib.cairo
Outdated
| assert( | ||
| !tokenized_bond.check_owner_and_operator(zero_balance_transfers), | ||
| 'Should fail for zero balance', | ||
| ); |
There was a problem hiding this comment.
Can you tell me why this check_owner_and_operator returns a falsey value here and this test pass?
There was a problem hiding this comment.
I just added this to ensure that accounts can't transfer tokens they don't own, even if they have special roles like being a minter.
tests/lib.cairo
Outdated
|
|
||
| start_cheat_caller_address(tokenized_bond.contract_address, NOT_MINTER()); | ||
| assert( | ||
| !tokenized_bond.check_owner_and_operator(transfers.clone()), 'Should fail as non-operator', |
There was a problem hiding this comment.
This isn't falsey because they are a non-operator
tests/lib.cairo
Outdated
| let receiver1 = setup_receiver(); | ||
| let receiver2 = setup_receiver(); | ||
|
|
||
| // Create two separate transfers, each with one destination |
There was a problem hiding this comment.
This comment contradicts the code below it
There was a problem hiding this comment.
oh, I need to make it as a separate destination. I forgot to change from the previous code.
There was a problem hiding this comment.
I will remove this comment too. Thanks for pointing it out.
tests/lib.cairo
Outdated
| } | ||
|
|
||
| #[test] | ||
| fn test_check_owner_and_operator_nonexistent_token() { |
There was a problem hiding this comment.
Why are we testing for a token that doesn't exist?
There was a problem hiding this comment.
I added this to verify that the contract properly handles actions on tokens that don’t exist/minted
tests/lib.cairo
Outdated
| fn test_check_owner_and_operator_as_non_owner_operator() { | ||
| let (tokenized_bond, _minter) = setup_contract_with_minter(); | ||
| let receiver1 = setup_receiver(); | ||
|
|
||
| let invalid_transfers = array![ | ||
| TokenizedBond::TransferParam { | ||
| from: receiver1, | ||
| to: array![ | ||
| TokenizedBond::TransferDestination { | ||
| receiver: receiver1, amount: TRANSFER_AMOUNT(), token_id: TOKEN_ID(), | ||
| }, | ||
| ], | ||
| }, | ||
| ]; | ||
|
|
||
| assert( | ||
| !tokenized_bond.check_owner_and_operator(invalid_transfers), | ||
| 'Failed for non-owner/operator', | ||
| ); | ||
| } |
There was a problem hiding this comment.
I don't understand this. The name of this test makes it sound like it's testing who calls the contract and then we have a invalid transaction with the same to and from address
There was a problem hiding this comment.
I know it's a bit confusing. I can remove this if you don't want it. I added this to check that a transfer will fail when the from is not the owner nor operator
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
…/tokenized-bond into feat/make-transfer
…/tokenized-bond into feat/make-transfer
| start_cheat_caller_address(tokenized_bond.contract_address, minter); | ||
| assert(tokenized_bond.check_owner_and_operator(transfers.clone()), 'Should pass as minter'); | ||
| assert(!tokenized_bond.check_owner_and_operator(transfers), 'Fail for different from address'); |
There was a problem hiding this comment.
This is returning false because the from address has no balance not because the caller is not the owner of the from address
| let erc_1155 = IERC1155Dispatcher { contract_address: tokenized_bond.contract_address }; | ||
|
|
||
| start_cheat_caller_address(tokenized_bond.contract_address, minter); | ||
| erc_1155.safe_transfer_from(minter, non_minter, TOKEN_ID(), TRANSFER_AMOUNT(), array![].span()); |
There was a problem hiding this comment.
it's hard to tell who is sending and receiving tokens in this function
No description provided.