Contract documentation #44
Conversation
docs/identityManager.md
Outdated
| * Note: In the case where Malory performs this action, an OwnerAdded event will be triggered, and thus Alice should be notified that this is occurring. As long as she realizes before `adminTimeLock`, she can delete both the new evil owner and the evil recovery. | ||
|
|
||
|
|
||
| ### Analysis of number of bad vs good owners |
There was a problem hiding this comment.
@oed not sure if we should include below. it's a lot of info that might not be to helpful (I'm also totally not confident in it :) ).
There was a problem hiding this comment.
At least, we should condense into a few useful cases.
There was a problem hiding this comment.
ok, will take a closer look tomorrow :)
There was a problem hiding this comment.
@naterush do you think what is described in scenarios is enough? I think it describes the most common use cases so I don't think we need the rest.
oed
force-pushed
the
feature/contract-docs
branch
from
9a1241c
to
39730a9
Compare
|
|
||
| ## Design | ||
| The interface of the MetaIdentityManager is the same as [IdentityManager](./identityManager.md) with some small changes to be able to use the [TxRelay](./txRelay.md). Namely adding sender as the first parameters of all authenticated methods, as well as a `onlyAuthorized` modifier. Also note that a user can both send meta-tx and regular tx to the MetaIdentityManager. | ||
|
|
There was a problem hiding this comment.
Maybe add the same line about "scenarios" and just point them to IdentityManager again.
| 2. nonce: prevents replay attacks. | ||
| 3. destination: necessary to know where tx is going :) | ||
| 4. data: necessary to know what the user is trying to do. | ||
| 5. relayer_address: prevents a not-really-feasible-but-maybe attack, where someone could front run, but not send enough gas when they do for the meta-tx to be processed, and then try and trick the user to convince them the transaction just failed, other than the meta-tx. They can then relay the transaction later. Assuming an honest relayer, this is not an issue. |
There was a problem hiding this comment.
Actually, this attack is much easier than I thought. Instead of having to convince the use of anything, here is how it works if the relayer_address was not signed:
Front run the transaction, and send it with enough gas to 1. get through the txRelay (aka, signature verification), but 2. not enough for the transaction to finish. Thus, the nonce would update, and then the users actual transaction would fail. This would be a very effective nusiance attack and it is stopped with relayers address being part of what is signed.
There was a problem hiding this comment.
I don't think that is the case since an oog-exception would roll back all state changes from that tx.
There was a problem hiding this comment.
@oed it's a tad more nefarious than that. The meta-tx contract does not throw when a sub call throws - which is a decision we made to make transactions as atomic as possible (aka, if a sub call throws, the relayer should not be able to replay the transaction again and again).
This means that even if a sub-call throws, the nonce is updated, and thus if we did not include the relayers address in the signed data, the nuisance attack described above would likely be possible.
There was a problem hiding this comment.
Throwing is not the same as oog right? I get that a throw in the subcall will still have the nonce updated. But iirc oog rolls back everything.
There was a problem hiding this comment.
@oed effectively, they are the same as a throw consumes all remaining gas. I believe that this means, in both cases (oog and throw), the nonce is updated.
There was a problem hiding this comment.
At least when these throws/oogs are in the sub-call.
There was a problem hiding this comment.
Oh, that's why you can specify gas for a subcall, so that you can have enough gas in the first call to be able to finish the tx if the subcall runs oog.
There was a problem hiding this comment.
@oed yeah. With that update to describe the attack, this is probably good to merge :~)
There was a problem hiding this comment.
I'm still confused by this. If we where to do an operation after the if(destination.call()) in the TxRelay contract there would be no gas to complete that operation. How does the EVM know that we are not doing anything after the if statement? If we run oog in the subcall there would be no gas left to complete the calling tx, or what am I missing?
docs/txRelay.md
Outdated
| Question: Why would anyone choose to replay as compared to censoring? | ||
| Answer: The transaction will appear on the blockchain (just as a failed transaction), and thus, to someone not paying attention, it may look like the relayer has not been compromised. | ||
|
|
||
| In our system we choose to trust the relayer since it is controlled by us. If a user don't want to trust the relayer they will still have the option of sending regular transactions. |
There was a problem hiding this comment.
Or the option of using a different relayer.
NOTE: This PR is based on and should only be merged after #43
Adding documentation for contracts, diagram for most used contract interactions, and documentation of the deployment process.