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

Test 1155 burning outside lock #197

Closed
hensha256 opened this issue Jun 8, 2023 · 3 comments
Closed

Test 1155 burning outside lock #197

hensha256 opened this issue Jun 8, 2023 · 3 comments

Comments

@hensha256
Copy link
Contributor

hensha256 commented Jun 8, 2023

Components

Lock and Call, 1155 Balances

Describe the suggested feature and problem it solves.

To burn an 1155 in exchange for pool balance, the onERC1155Received hook burns and accounts the tokens. However this function doesnt have a lock on it. I think the accountDelta function should revert if you try to transfer 1155s into the manager outside of a lock (as the lockedBy.length should be 0). However we should test this to make sure theres nothing weird you can do by calling onERC1155Received outside of a lock.

Secondly what if an unsafe transfer is done, so the onERC1155Received hook isnt called? Does the pool get a balance that then cannot be burnt?

Describe the desired implementation.

Tests for the above edgecases, and any others we can think of, to check we've covered any unplanned behaviour.

Describe alternatives.

No response

Additional context.

No response

@hensha256 hensha256 added the good first issue Good for newcomers label Jun 8, 2023
@abhithory
Copy link

Hi @hensha256

So as i am able to understand:

  • in "onERC1155Received" function there is no lock which means everyone can call this function.
  • "_accountDelta" function is called inside this function. so in this function, we should add a revert when "delta == 0" that is currently it is just a return.

So Now:

we should test that:

  • nothing happens wrong when we call "onERC1155Received" directly outside of a lock.
  • also check conditions when an unsafe transfer is done. (conditions like:- the balance gets updated etc..)

please tell me...
Am i thinking right?
also if there is anything wrong or missing something then also tell me......


Also please tell me these things. i am not able to understand this:---

  • what do you mean by unsafe transfer?? when it happen and how? is it when the "onERC1155Received" hook is not get called?

  • Which revert should I add when " delta == 0 "?

@ZehnSh
Copy link

ZehnSh commented Jul 5, 2023

Hi I want to work on this issue
Thanks

@zhongeric
Copy link
Contributor

Closed by #379

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

No branches or pull requests

5 participants