Check that the twin address is a contract #653
Conversation
zajck
changed the title
zajck
changed the title
zajck
force-pushed
the
transfer-twins-check-code
branch
from
5d6989a
to
1c61a4f
Compare
|
|
||
| if (tokenType == TokenType.FungibleToken) { | ||
| // ERC-20 style transfer | ||
| data = abi.encodeCall(IERC20.transferFrom, (seller.assistant, sender, twin.amount)); |
There was a problem hiding this comment.
I wonder why we decided to the change this one and not the subsequent similar calls on lines 742 and 751
There was a problem hiding this comment.
abi.encodeCall was introduced in solidity 8.11. It's a safer alternative to other abi encoders since it checks types at compile time and it's suggested way to use.
We cant use the same on lines 742 and 751 because of the not yet resolved issue with overloaded functions and their selectors: ethereum/solidity#3556
| twin.tokenId++; | ||
| } else { | ||
| // Token transfer order is descending | ||
| tokenId += twin.supplyAvailable; |
There was a problem hiding this comment.
This is just an aside, by i prefer the previous syntax here, it was more verbose, i like verbose as I have a simple internal parser ... but anyways, no need to change
There was a problem hiding this comment.
This reason for this change wasn't even the syntax itself, but I just wanted to remove reading storage twin.tokenId.
tokenId = tokenId + twin.supplyAvailable would be ok for me, too, but now that it's merged already, I won't change it.
| ) | ||
| ); | ||
| ); | ||
| } |
There was a problem hiding this comment.
I guess also, that there is no way for us to have an unsupported TokenType at this point? If it is possible, would it be sane to have an else clause and to throw a "unknown token type" kind of revert ? If so, we could always create an issue for the next release
There was a problem hiding this comment.
there isn't. tokenType is defined when creating a twin, and if a seller passes a value that is not present in the TokenType enum transaction will be reverted.
There was a problem hiding this comment.
OK, thanks for the reply @anajuliabit
| ) | ||
| ); | ||
| ); | ||
| } |
There was a problem hiding this comment.
there isn't. tokenType is defined when creating a twin, and if a seller passes a value that is not present in the TokenType enum transaction will be reverted.
Fix #616
This is branched from #652 which should be merged before this.