Make sure that MUW is only capable of calling approve/setTransferProxy for sending ships (and safeTransferFrom to receive)

[vvisigoth]

There are two ways to transfer a ship via the contract, one which doesn't require a double confirmation, and one that does. We want to enforce on the UI side that double confirmation is required. 

Via @msutherl

[vvisigoth]

@msutherl @Fang- Right now it looks like we're just getting ready for ERC721, no? So this is a cautionary issue?

[Fang-]

To give some background and clarify on what needs to be done here:

Losing address space is Bad, since we can't just print more of it. An expected source of lost address space is transfers to Ethereum addresses that are either ERC721-unaware contracts, Urbit-unaware/uninterested people, or just plain incorrect addresses.

ERC721's safeTransferFrom() function takes care of the dumb contract case, but does nothing to keep you from sending to, say, an Ethereum address that nobody has the keys to.

So we want a mechanic that makes sure the recipient is aware of the address space it received. And look, ERC721 (and thus the Constitution) already comes with such a thing! You might have seen how we can approve()/setTransferProxy(). This allows the specified address to transfer ownership of one of our assets.

While we can't completely close off the "directly transfer ship to an address" part of the contract (not without breaking ERC721 support at least), we can make sure our UI doesn't offer you much choice but to use the safer option.

So, the UI flow for transferring a ship to someone else would look something like this:

1. A sends ship X to B
2. A sees the transfer is pending and can cancel it at any time
3. B opens MEW, sees it can gain ownership of ship X
4. B clicks the button to accept the transfer

With the corresponding logic:

1. A calls approve(B, X)
2. A can call approve(0, X) as long as it still owns it
3. MEW fetches a list of ships the user (B) has been approved for (this is a contract-side TODO, I'll report back)
4. B calls safeTransferFrom(A, X, B) (or alternatively transferShip(X, B, true). Sticking to using the ERC721 interface when we can might be nice, but it really doesn't matter that much.)

[Fang-]

transferShip()'s last argument is a bool _reset. Setting it to true causes public keys, continuity, proxies and claims to be reset for the transferred ship. This is always set to true when using the ERC721 interface to transfer.

So, to support the "I'm just transferring to another of my own addresses" case, we may want to use transferShip() for point 4 above, and make the reset flag a checkbox that's on by default.

[Fang-]

The reverse lookup for transfer proxies got put in here. On sign-in, you'll want to get data from getTransferringFor(our-eth-address) and display the ships listed there as "pending transfers" that need to be accepted, which means doing transferShip(that-ship, our-eth-address, true) as described above.
I'll put in the same thing for spawn proxies soon, which will be much less common but is still worth accounting for.

Logic for spawning ships got changed to also follow a similar withdraw pattern. This change has a number of implications, so let's take this part by part.

New spawn logic is as follows: instead of activating the ship and making the target address the owner, we don't activate the ship, and approve the target address for transfer of the ship. The ship is activated as soon as it is transferred. To make sure this "pending transfer" can be cancelled, the ship needs an owner. We set this to be the parent ship's owner.
(If the spawn target's address is the same as the caller's, we follow traditional spawn logic and complete the transfer right away.)

The above allows ships to be in a state where they do have an owner, but aren't active. This wasn't possible before, but now it is. Ships in that state should be considered as "pending spawns".
Operations you can do if you own an inactive ship: transferring, (un)approving transfers.
Operations you can do if you own an active ship: the above, plus everything else. Nothing changes here.
This means that "everything else" will need an additional check to make sure the ship is actually active, before displaying/allowing those actions. We probably want to display "pending spawn" ships differently in some way.

No other contracts have changed semantically because of this, aside from the "ships needs to be owned and active" thing.

Does this give a good idea of the changes that need to be made here? Would be happy to clarify, answer questions, hop on call, etc.

cc @bencalder because idk if you're getting auto-notified of these or not (:

[bencalder]

@Fang- Sounds good. On sign-in I will call getTransferringFor(our-eth-address) and getOwnedShipsByAddress(our-eth-address) in order to display all ships pending transfer as well as active owned ships.

And yes, please continue the username mentions, that's how I'm getting notified :)

[Fang-]

Per this commit you can now call getSpawningFor(our-eth-address) to get a list of ships the user can freely spawn from.