When I look at our bridge contract, I find we use ERC677, I am not sure how to implement it, but I think it will use approve() in the transferAndCall function and transferFrom() in the receiver.onTokenTransfer() function, but I did not find it in our contracts, so how do we implement it?

@akolotov please take a look

Hi @Skyge! Could you clarify your question I think I did not catch it fully?

BTW, we have our implementation of the ERC677 token which is already being used in the POA and WETC bridges.

@akolotov Hi, thanks for your replying, I think ERC677 mainly achieve approve() and transferFrom() in one transaction, I find we use a different way, we just use transfer() function, but if the receiver address is a contract address, we will emit an extra event. So what I mean is that our ERC677 contract is different from the original proposal named EIP677.

@Skyge the main reason for the EIP677 is to notify the contract in one transaction that it received some tokens. It is achieved by the call onTokenTransfer within of transferAndCall. This is exactly the way we use the token with the bridge: the user calls transferAndCall with the bridge contract address as the receiver of tokens, the method transfers tokens to the bridge contract account (it means that the tokens now are locked on the bridge) and calls onTokenTransfer from the bridge contract so the bridge could initiate the process of relaying tokens to another chain.

The method transferAndCall probably could be implemented in the way as you described: it could call approve (if it is not declared as external), pass the flow control to the recipient contract by onTokenTransfer and the recipient contract will call transferFrom in its turn. But as it was mentioned in https://github.com/ethereum/EIPs/issues/677#issuecomment-319862841 it could lead to re-entrance attacks.


@akolotov Really thanks for your clear explaining, but if we implement as what I said, how to achieve re-entrance attacks?

It depends on a specific implementation. Could you provide a code snippet with the contracts as so we can say if an re-entrance attack is possible there.

@akolotov Such like this: https://gist.github.com/Skyge/d0d69a2fe9ed8b39521716cce6399399

Thanks for the example. In this particular case I do not see how re-entrance attack could be implemented.

But for me, the pair approve and transferFrom as you used them in transferAndCall is redundant: it modifies the state serving the approval amount twice (to amount first and then to zero) and emits Approval event twice too without introduction of evident benefits. Moreover emitting Transfer in transferAndCall is odd since it will be done by transferFrom. Also your implementation assumes that other contracts which have a fallback method will also use transferFrom to do actual transfer but it is not what ERC677 standard declares.

I found yet another probably undesired behavior for your code. If someone will approve some amount of tokens for the ERC677ReceiverTest contract (e.g. by mistake), anyone could call onTokenTransfer to lock this amount of tokens on the ERC677ReceiverTest contract.

I agree with @Skyge that maybe next version of the ERC20 to ERC20 token bridge should have approve + transferFrom vs single transfer in order to record the limits and check if the tx will be successful

If we use this pattern, at first, we will approve the contract a big enough balance, which is usually is -1, and then the dapp contract can call transferFrom(), as for two approval() event, why? I think we can just emit once.
Yeah, you are right, I should make a modifier canCall() in the ERC677ReceiverTest contract, that means only this contract can call this function.

Sorry, but I do not understand why you need to use approve at all in this code. The code you presented is doing the following atomic action:

  • allow to transfer C+X tokens;
  • transfer X tokens;
  • modify the allowance to transfer C-X tokens.

Why do you need to the first and the third operations?

Oh, I see, I know your meaning at now, yeah, you are right. Thanks for your patiently explaining again!

1 Like