Add alternative receiver for transfer in native to erc20 mode #302
Conversation
# Conflicts: # contracts/upgradeable_contracts/InitializableBridge.sol
# Conflicts: # contracts/upgradeable_contracts/ERC677BridgeForBurnableMintableToken.sol # contracts/upgradeable_contracts/amb_erc677_to_erc677/BasicAMBErc677ToErc677.sol # contracts/upgradeable_contracts/amb_erc677_to_erc677/ForeignAMBErc677ToErc677.sol # contracts/upgradeable_contracts/amb_erc677_to_erc677/HomeAMBErc677ToErc677.sol
| bytes /*_data*/ | ||
| ) internal { | ||
| contract ERC677BridgeForBurnableMintableToken is ERC677Bridge, OtherSideBridgeStorage { | ||
| function chooseReceiver(address _from, bytes _data) internal view returns (address recipient) { |
There was a problem hiding this comment.
seems that we have the same method in amb_erc677_to_erc677/BasicAMBErc677ToErc677.sol. Does it make sense to have a separate module for it?
There was a problem hiding this comment.
Moved definition of the method fd80024
| @@ -22,7 +22,8 @@ contract ForeignBridgeNativeToErc is | |||
| uint256 _requiredBlockConfirmations, | |||
| uint256[] _homeDailyLimitHomeMaxPerTxArray, // [ 0 = _homeDailyLimit, 1 = _homeMaxPerTx ] | |||
| address _owner, | |||
| uint256 _decimalShift | |||
| uint256 _decimalShift, | |||
| address _bridgeOnOtherSide | |||
There was a problem hiding this comment.
is my understanding correct and the scenario approve-then-relayTokens is not supported in the bridge mode. Do you think it make sense to keep consistency and support it for all bridge modes where erc677 tokens are intended to be used? If yes, I will create a new issue.
There was a problem hiding this comment.
You are correct, approve-then-relayTokens was only added for the modes where erc677 is not present in order to be able to specify an alternative receiver. I think that since erc677 can specify a different receiver using transferAndCall method, relayTokens is not needed for those cases because it will be a duplicated functionality and from user perspective requires two transactions instead of one so I think transferAndCall will be always preferred
There was a problem hiding this comment.
right. but consider a following situation: a user knows that on the bridge A he/she can transfer tokens by pairing approve and relayTokens. Next, the same user tries to transfer tokens through the bridge B but fails. In both cases he/she considers the token contract as erc20 compatible. But in one case it works and does not work in another. I assumed this consistency when raised the comment initially. I understand that's it is not mandatory to have but asking your opinion whether we can suggest the user this level of usability in the future.
There was a problem hiding this comment.
Thanks for the explanation, in that case keeping consistency and allowing the users to interact in the same way with all the bridges is a nice usability feature, let's create a new issue for this.
Closes #297