Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transcoder withdrawal address #4

Open
yondonfu opened this issue May 16, 2018 · 3 comments
Open

Transcoder withdrawal address #4

yondonfu opened this issue May 16, 2018 · 3 comments

Comments

@yondonfu
Copy link
Member

yondonfu commented May 16, 2018

At the moment, transcoders use a single address for the following purposes:

  • As the on-chain identifier for the transcoder
  • The associated private key signs transactions to invoke contract functions such as BondingManager.reward(), JobsManager.claimWork() and JobsManger.verify() that validate that the sender address is the transcoder's identifier address
  • As the withdrawal address for any LPT earned as rewards and ETH earned as fees

Transcoders can instead use one address as their identifier address (so that contract functions can validate that the transcoder is the sender) and use one address as their withdrawal address.

There are a number of benefits that come with allowing a transcoder to specify a withdrawal address that is distinct from its identifier address

  • The associated private key for the identifier address can be "hot" and the associated private key for the withdrawal address can be "cold". The hot key can hold just enough ETH to submit transactions associated with being an active transcoder and be actively used to sign transactions by an automatic process. The cold key can hold larger amounts of ETH/LPT and can be secured by a hardware wallet since it does not need to be actively used. Note: it will nonetheless be crucial for a transcoder node operator to properly secure the hot key since a compromised hot key can allow an attacker to cause the transcoder node operator to be slashed. However, with this hot key and cold key setup, any of the transcoder node operator's withdrawn ETH/LPT or reserve funds will be safe in the event of a compromised hot key
  • The withdrawal address can be associated with a contract that specifies rules for the use of withdrawn ETH/LPT
    • The contract could be a 2-of-3 multisig wallet - 3 parties could agree to be joint owners of a
      transcoder node and designate one party to be the transcoder node operator with any earnings
      generated being transferred into this multisig wallet. Multisig wallet funds could then be spent if 2 of 3
      parties agree.
    • The contract could be an Aragon DAO - a group of community members
      could agree to be members of the DAO which owns the transcoder node and designate one party to be the transcoder node operator with any earnings being transferred into the DAO. An initial
      implementation of the DAO might use a simple voting application to allow community members to
      authorize the use of DAO funds. The DAO could be upgraded to use alternative applications such as
      liquid democracy to govern the use of DAO funds
    • The contract could define payout logic for a staking service such as 1Protocol

From an implementation point of view this would require an additional withdrawalAddress in the transcoder struct in the BondingManager contract. This value of withdrawalAddress could be specified when invoking the BondingManager.transcoder() function. (One note here is that the BondingManager.transcoder() would become even more overloaded in this case because it is used not only to register as a transcoder, but also for changing rates and now setting a withdrawal address. It might be worth splitting up the function into two separate functions, one for registering with a withdrawal address and one for setting rates).

BondingManager.withdrawStake() would check if the sender has a withdrawalAddress as a transcoder - if so, the function will ask the Minter to transfer LPT rewards to withdrawalAddress, if not the function will ask the Minter to transfer LPT rewards to the sender address. BondingManager.withdrawFees() would work similarly.

BondingManager is delegate proxied meaning any changes during an upgrade will have to be storage layout compatible. At the moment, the addition of withdrawalAddress to the transcoder struct should be acceptable because the transcoder struct is only used in a mapping and not in any arrays and we would be adding an additional storage variable and not deleting or replacing any existing ones (we would need to make sure it comes after all existing variables in the transcoder struct!).

@dob
Copy link
Member

dob commented May 16, 2018

Related concept: is there any merit in the "cold" address (withdrawl address), being able to submit a tx which replaces the "hot" address? The reason is that the hot address could be easily compromised...but if so then maybe only short term damage could be done if replaced quickly.

Don't want to derail the discussion around the above, but I guess this issue is a place for discussion before actually submitting the PR proposal.

@j0sh
Copy link

j0sh commented May 16, 2018

I like it.

Another note about overloading BondingManager.transcoder() . We might end up putting additional fields in there as well, eg a URI . Does this mean we should have a separate setter function for each field? Or is a more generalized approach possible without increasing gas too much?

@yondonfu
Copy link
Member Author

yondonfu commented May 17, 2018

is there any merit in the "cold" address (withdrawl address), being able to submit a tx which replaces the "hot" address? The reason is that the hot address could be easily compromised...but if so then maybe only short term damage could be done if replaced quickly

Another potentially simpler alternative to minimize damage would be to allow the withdrawal address to invoke the BondingManager.unbond() and BondingManager.withdraw() functions to get funds out of the contract. The complication here is that the protocol currently allows bonding from the unbonding state, so an attacker with access to the hot key could still force the transcoder to remain bonded by invoking BondingManager.bond(). A possible solution here is disallowing bonding from the unbonding state if you were a registered transcoder.

We might end up putting additional fields in there as well, eg a URI . Does this mean we should have a separate setter function for each field?

Not necessarily - there could be a single separate setter function for updating rates while BondingManager.transcoder() allows for registration as a transcoder in addition to setting the initial rates and withdrawal address. As for the URI, it would be nice if we can use a separate contract that supports lookups for additional metadata (e.g. custom ENS registry + resolver that would allow transcoders to register not only human readable names, but also metadata such as URIs) associated with an address (and the transcoder's identifying address would be the only address permitted to make updates) rather than including additional read only information in this existing contract

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants