v4-core icon indicating copy to clipboard operation
v4-core copied to clipboard

Test 1155 burning outside lock

Open hensha256 opened this issue 1 year ago • 2 comments

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 avatar Jun 08 '23 13:06 hensha256

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 "?

abhithory avatar Jun 14 '23 04:06 abhithory

Hi I want to work on this issue Thanks

ZehnSh avatar Jul 05 '23 09:07 ZehnSh

Closed by #379

zhongeric avatar Nov 14 '23 20:11 zhongeric