Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Q: who is the proof producer for WithdrawResults? #32

Closed
olga24912 opened this issue May 9, 2023 · 2 comments · Fixed by #65
Closed

Q: who is the proof producer for WithdrawResults? #32

olga24912 opened this issue May 9, 2023 · 2 comments · Fixed by #65
Assignees

Comments

@olga24912
Copy link
Collaborator

For finalizing withdrawing Eth on Ethereum side in EthCustodian contract we check the proof producer of the WithdrawResult: https://github.com/aurora-is-near/eth-connector/blob/master/eth-custodian/contracts/ProofKeeper.sol#L74

According to that line, where we create the address for depositing Eth to Aurora, the nearProofProducerAccount is expected to be nearSDK ~ aurora-engine (aurora and etc): https://github.com/aurora-is-near/eth-connector/blob/master/eth-custodian/contracts/EthCustodian.sol#L68

We have two ways to withdraw Eth from Near: call the withdraw function at aurora-engine(it redirect the function call to the engine-withdraw function in aurora-eth-connector), or call the withdraw function directly at aurora-eth-connector. I am not sure who will be the WithdrawResult producer in the first case, but in the second case it is aurora-eth-connector. I guess in the first case it is aurora-eth-connector as well, not the aurora-engine. https://github.com/aurora-is-near/aurora-eth-connector/blob/master/eth-connector/src/lib.rs#L504

In that case, we broke the backward compatibility because some contracts (for example EthCustodian) can expect that the WithdrawResult is produced by aurora-engine.

@mrLSD
Copy link
Collaborator

mrLSD commented May 9, 2023

In Engine, we have precompiles: ExitToEthereum, ExitToNear. It's about withdraw producers.

And for Engine we have special method in aurora-eth-connector - engine_withdraw. In the future Engine precompiles will call directly aurora-eth-connector.

I can't see where logic is broken.

@mrLSD mrLSD self-assigned this May 9, 2023
@karim-en
Copy link
Collaborator

karim-en commented May 16, 2023

@mrLSD The problem is that on eth side we have just one field to store the near-side account , it is used for both withdraw and deposit actions.
The nearProofProducerAccount will be changed from aurora to a new account eg. eth-connector.aurora, so, in this case, the method depositToEVM produces a message eth-connector.aurora:ETH_ADDRESS here.

For backward compatibility, it is better to handle this case by adding a check on the eth connector near-side:

correct_receiver_id = if receiver_id == current_account_id() {
  return parent of current_account_id() or EVM account id from storage (`aurora`), or hardcoded `aurora`.
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants