Open dob opened 8 years ago
Currently, the endAuction
contract method has a weakness in that it calls setOwner()
on the Asset
contract, and if that call uses all the gas and fails, then the auction can be stuck in a state where the winner can't get their asset or their money back. If there were a standard, verifiable, implementation of setOwner
as suggested by this issue, then this would be defended against, but as of now it seems like a security hole.
How about using the mechanism of a "Safe Remote Purchase" to validate the sale as in http://solidity.readthedocs.io/en/develop/solidity-by-example.html (maybe with an escape hatch whereby [1 or 2] parties lose X% of the deposit etc.)?
@rmerom I like that as an option. It still requires the buyer to be familiar enough with the contract of the asset they're purchasing to have confidence that when they call confirmReceived()
, the item has actually been fully transferred to them in the original asset contract...which is kind of the same challenge we have with trusting the implementation of setOwner
to do this. And if they have that trust, then by using the Asset interface the auction contract can do this transfer for them without them having to go through the safe remote purchase procedure. But the nice thing about adding safe remote purchase would be that people could auction assets that haven't conformed to the Asset.sol interface. Good suggestion, thanks.
On a related note, on the issue of trusting remote contract calls, we have some ideas for a decentralized social transaction attestation platform, built on identity and reputation that we're thinking about hacking on soon ;)
@dob yes, i think the advantage of using the Safe Remote Purchase paradigm is that it's easier for non-techies to validate. Suppose, for example, that I want to buy an Ethereum-based asset of a game. As an ordinary person, I may not have an idea what all these "contracts" mean or how to verify them, but I surely know whether I got the asset or not.
I'm curious about the decentralized social transaction attestation platform. I myself am trying to come up with a reasonable Automated Bug Bounty paradigm to help high-stake contract authors build public confidence in their contracts, on top of the more traditional security audits.
One option is an Escrow situation for the Asset itself. Instead of just attempting to call setOwner()
at the end of the auction, the seller must give up ownership of the Asset to the Auction contract at the start of the auction, proving ownership can be transferred?
verifyOwnership
is called on the Auction contract, which verifies using the owner()
method that the Asset is currently owned by itself, and then uses uses the setOwner()
to transfer the Asset to a known "bounce-back" contract (a contract that all it does is transfer ownership back to whomever gives it something), and verifies the ownership changes properly.@MidnightLightning Thanks, that's close to how it works now actually. The seller doesn't need to create a new auction contract, as he can use the one that's already deployed, but he does need to call setOwner()
to transfer ownership to the auction contract itself prior to the auction being activated. The auction contract then takes care of transferring it back to the owner in the case of an unsuccessful auction, or transferring it to the winner in the case of a successful one.
Ah, great; I missed that detail that the Asset is already owned by the AuctionHouse for the duration of the auction. As I've been musing on this more, maybe what's needed is a pre-auction escrow contract? So, the seller transfers ownership to the escrow contract, and triggers creating a new auction at the AuctionHouse. The AuctionHouse can verify that the Asset is owned by the escrow contract, and then trigger the escrow contract to send the Asset to the AuctionHouse. The escrow contract uses the setOwner()
then, and either the escrow contract or the AuctionHouse can verify that it worked to send the asset to the AuctionHouse. That way it can be trusted for the rest of the auction that the setOwner()
function of the Asset will transfer ownership, even if it is non-standard?
If the escrow contract fails to transfer the asset to the AuctionHouse (setOwner()
used up all the gas, as you suggested as a potential problem), the auction then won't even start, and wouldn't put any buyers at risk. There'd need to be some sort of escape mechanism to transfer the Asset back to the seller in that case, which the escrow contract could handle.
Yah I like some things about the direction of this idea. The "escrow contract" is probably a wrapper on top of the ownership transfer mechanisms of other assets. I still think you'd unfortunately need many escrow contract implementations - one for each of the popular assets with different interfaces. Kind of like the way the WrappedEth ERC20 token is just a wrapper around Ethereum so that it can be traded on Maker's exchange. And then each of these would have to be vetted to make sure it's implemented correctly.
But at least the AuctionHouse contract could use the standard setOwner()
interface, and anyone that wanted to add support for a certain type of asset being auctionable would have a clear, easy process to follow.
ERC 721 (https://github.com/ethereum/EIPs/issues/721) addresses this, so AuctionHouse should update to support these assets.
Any item that adheres to the Asset.sol spec is auctionable, however we aren’t sure how to guarantee other than via social inspection and proof, that the underlying implementation of
owner()
andsetOwner()
actually transfer full semantic ownership of the asset to the winning bidder. In the fungible world, ERC20 has emerged as a standard, however in the non fungible world there can’t really be a standard as each asset has unique attributes, metadata, and functionality. There’s no standard bytecode to check against like there is with ERC20. Any suggestions would be appreciated here.