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

Remove burnResult function from ERC20Locker #25

Closed
alexauroradev opened this issue Feb 15, 2021 · 4 comments · Fixed by #26
Closed

Remove burnResult function from ERC20Locker #25

alexauroradev opened this issue Feb 15, 2021 · 4 comments · Fixed by #26
Assignees
Labels
bug Something isn't working P1

Comments

@alexauroradev
Copy link

alexauroradev commented Feb 15, 2021

https://github.com/near/rainbow-token-connector/blob/master/erc20-connector/contracts/ERC20Locker.sol#L43-L47

@alexauroradev alexauroradev added bug Something isn't working P1 labels Feb 15, 2021
@alexauroradev alexauroradev self-assigned this Feb 15, 2021
@alexauroradev alexauroradev changed the title Remove burn_result function from ERC20Locker Remove burnResult function from ERC20Locker Feb 16, 2021
@sept-en
Copy link
Contributor

sept-en commented Feb 16, 2021

Assign me to this, please.

sept-en added a commit to sept-en/rainbow-token-connector that referenced this issue Feb 16, 2021
@mfornet
Copy link
Contributor

mfornet commented Feb 16, 2021

What was the use case of the burn function?

It seems to me that it should have only work for a parallel connector transferring bridged NEP21 from ETH to native NEP21 in Near)

@sept-en
Copy link
Contributor

sept-en commented Feb 16, 2021

What was the use case of the burn function?

It seems to me that it should have only work for a parallel connector transferring bridged NEP21 from ETH to native NEP21 in Near)

It seems to be redundant and not used anywhere. Moreover, yesterday on the call @abacabadabacaba mentioned that this one could be a bug. As the _parseProof which is called inside consumes the proof as well. So any third party can burn produced proof in ERC20Locker (i.e. this specific proof become "used" and is not accepted anymore) and the appropriate amount of eth will be "locked" forever.

@alexauroradev
Copy link
Author

@mfornet Illia mentioned that this is a leftover from the refactoring / debugging, no clear evidence why this appeared. The initial code was not containing this.

This is a clear bug that should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants