search

Keydonix / liquid-long

The Unlicense
8 stars 1 forks source link

Close cdp #108

Closed epheph closed 5 years ago

epheph commented 5 years ago

"what happens if msg.sender isn't who we expect" are you thinking maliciously?

MicahZoltu commented 5 years ago

Malicious or accident. I just want to make sure we fully understand what happens in either case so we can be sure we are making an informed decision to not have a require(msg.sender.codehash == 0xabcd) (or similar)

epheph commented 5 years ago

I think the way we should think about it is: you can construct any transaction on-chain, using a custom smart contract or DSProxy, to transfer a CDP to LiquidLong and call it with any arbitrary recipient. We offer a common (intended to be static) function that is intended to be delegated, but any function which accomplishes those tasks will do.

I'm not concerned about the closeCdp() entrance into the DS proxy, since that is just the common function AND the entrance into DSProxy(msg.sender) is the very first thing that happens (concerning reentrancy).

The more general closeGiftedCdp() operates assuming it has already been given a CDP (although it could be called by anyone if LiquidLong DOES end up legit owning a CDP somehow, with an arbitrary _recipient). The .call() at the end isn't concerning, since it is the very last thing to happen in that function.

I would say that the most awkward part of this is that we just assume that we should give() the CDP back to msg.sender. Perhaps another argument to closeGiftedCdp could be added to be the original owner of the CDP? I could imagine a contract someone else wrote to replace the closeCdp() function that never intends to own CDP itself?