VoteProxy2

[nmushegian]

Single forward call per call. GOV can be transferred directly to the contract - in fact it must be, there is no way to pull from any address.

Function calls require explicit quantities - necessary to add *all functions?

Massively simplify factory proxy "tracking" - voteproxy1 factory is an anti-pattern.

[chrisbradbury88]

Hey @nmushegian , thanks for this! This was actually something we were looking to do very soon anyway. We also have a couple of other changes we need to make to the vote proxy, which we will do very soon now too. If it gets the ok from some of the devs that originally wrote this proxy, we can certainly merge this into master.

With regards to updating vote.makerdao.com, we don't want users to have to migrate twice, so given we know the other changes we need to put in, we will get these done and update the vote proxy just once when all the changes have been made.

[chrisbradbury88]

@nmushegian With regards to what we were just discussing, as far as I can see from this PR the user still only designates a HOT and COLD wallet, and the MKR can only be returned to the COLD wallet which was set up initially (regardless of where the MKR actually came from).

What we are also looking for is being able to assign a different return address for the MKR, which is the only address to which the MKR can be sent back to.

[MicahZoltu]

What we are also looking for is being able to assign a different return address for the MKR, which is the only address to which the MKR can be sent back to.

That is what the cold wallet is.

[chrisbradbury88]

What we are also looking for is being able to assign a different return address for the MKR, which is the only address to which the MKR can be sent back to.

That is what the cold wallet is.

So if a user sets up a vote proxy with a cold wallet of 0xABC....... could we use the current contract to say MKR can only be withdrawn to 0xDEF.... for instance?

[nmushegian]

nikolai M 11:22 AM
isn't that the behavior of the new one? o.O
chrisb M 11:22 AM
I thought what you had done was just able to 'Push' MKR to the vote contract...
chrisb M 11:24 AM
Disclaimer Not a dev, product guy - can't actually read solidity code. Was going off your comments above.

[kjekac]

What we are also looking for is being able to assign a different return address for the MKR, which is the only address to which the MKR can be sent back to.

That is what the cold wallet is.

So if a user sets up a vote proxy with a cold wallet of 0xABC....... could we use the current contract to say MKR can only be withdrawn to 0xDEF.... for instance?

No, the current code only supports withdrawal to the cold wallet, but the MKR can be deposited from any address. So your use case should be supported, it's simply a matter of making the deposit from an address distinct from the cold wallet.

[MicahZoltu]

So if a user sets up a vote proxy with a cold wallet of 0xABC....... could we use the current contract to say MKR can only be withdrawn to 0xDEF.... for instance?

@chrisbradbury88 In this scenario, 0xDEF is the cold wallet. Really all the cold wallet is is "where MKR is withdrawn to". I'm not sure what 0xABC is in your scenario, but it isn't the cold wallet (because by definition, the cold wallet is where the MKR is withdrawn to, which is 0xDEF in your example).

[nmushegian]

No, the current code only supports withdrawal to the cold wallet, but the MKR can be deposited from any address.

But you can transfer MKR in no matter what... that was one of the main issues with voteproxy1

I can see the argument for adding a way to pull from the cold address (requiring an approval from the cold address), but I don't see why on earth you would want to stop people from being send in from any address to begin with. That's a token behavior level change.

[nmushegian]

Let's not get distracted with red herrings - are there any actual issues? Assume for now we're satisfied with the use cases this enables

[kjekac]

No, the current code only supports withdrawal to the cold wallet, but the MKR can be deposited from any address.

But you can transfer MKR in no matter what... that was one of the main issues with voteproxy1

I can see the argument for adding a way to pull from the cold address (requiring an approval from the cold address), but I don't see why on earth you would want to stop people from being send in from any address to begin with. That's a token behavior level change.

Yeah this was not criticism of the code, I just tried to clarify why the code is fine even when taking @chrisbradbury88's comment into account.

are there any actual issues?

Not as far as I can see.

[nmushegian]

I nuked voteproxy1 from this branch wholesale after seeing the factory, rather than trying to finish updating it for the latest compiler. If anyone wants to update voteproxy1 for whatever reason, you can continue from f3a9cd1

[gbalabasquer]

The proxy looks really clean to me. The only things to be defined IMO are:

• missing vote(address[]) any reason to not add it?
• grouped functions like: releaseAll, lockAll, lockAndVote, freeAndRelease. It is true the hot wallet could be used with a ds-proxy and not worry about grouped actions, but it is not how currently the governance dashboard works. Maybe we can live without them and integrate ds-proxy later.
• shouldn't we also have some function that pulls gov token? I mean in this case we are always forcing to make 2 txs. Excepting of course we use ds-proxy.
• About the `VoteProxyFactory. I understand the new code is how should be from a correct contract design perspective. However logs are not always available or working properly, so we need better ways for the UIs to relay in certain information. Otherwise we should have some offchain service in place for this coming release.
  Also I remember that the whole logic of linking the two accounts was deeply related to how the governance dashboard works. @jparklev can probably expand better on this.

[nmushegian]

missing vote(address[]) any reason to not add it?

Complexity for a temporary solution. You can create a slate with any low stakes key, and most votes don't create new slates. I won't object if someone adds it back, I guess.

grouped functions like: releaseAll, lockAll, lockAndVote, freeAndRelease. It is true the hot wallet could be used with a ds-proxy and not worry about grouped actions, but it is not how currently the governance dashboard works. Maybe we can live without them and integrate ds-proxy later

Generally UIs need to bend to contract design constraints, but these particular compositions are fine, especially for temp solution

shouldn't we also have some function that pulls gov token? I mean in this case we are always forcing to make 2 txs.

You still have to do an approval transaction, and I was thinking about the cold address when I decided not to add it - any extra transaction is bad for the cold address, and you want to be able to transfer in. But I can imagine pulling from the hot address, so you might as well enable for both. I'm ambivalent but wouldn't oppose it.

However logs are not always available or working properly, so we need better ways for the UIs to relay in certain information

That's pretty wild, logs are designed to be more available than storage for thin clients / at scale. Nevermind, it's not more available than present-state, but still. Not sure what to say about this except, this is a problem that will go away, while contracts are forever.

Also I remember that the whole logic of linking the two accounts was deeply related to how the governance dashboard works.

Again UIs need to bend to contract design constraints and not versa. But I can't say more until I know what the supposed constraints they want on the UI side is.

[gbalabasquer]

You still have to do an approval transaction, and I was thinking about the cold address when I decided not to add it - any extra transaction is bad for the cold address, and you want to be able to transfer in. But I can imagine pulling from the hot address, so you might as well enable for both. I'm ambivalent but wouldn't oppose it.

Right, but the approval once is done allows you to do repeatedly locks without a previous transfer.
We might also allow to pull from any wallet. I can imagine a user with a schema of 3 wallets: Metamask (Hot) for voting, paper wallet (cold) for withdrawing every locked MKR, HW for operating in exchanges. Then the user could monthly buy new MKR with the HW and execute a unique tx for locking via the hot wallet but pulling the MKR from the HW.

That's pretty wild, logs are designed to be more available than storage for thin clients / at scale. Not sure what to say about this except, this is a problem that will go away, while contracts are forever.

I'm not sure if that happens in reality. We never had significant issues bringing existing contract data but several reaching logs (not referring about historical from previous block numbers) .

Again UIs need to bend to contract design constraints and not versa. But I can't say more until I know what the supposed constraints they want on the UI side is.

I kind of agree and disagree. From a smart contract developer perspective I can say you are right, nothing is better than a clean structure without any redundant data.
However at the end they need to be used by applications and users. In certain occasions I endorse some redundancy in the data structure if it really simplifies the interaction experience (not only for the official UIs but also for community development).

[MicahZoltu]

@gbalabasquer It sounds like you want a scheme that involves more than hot and cold? In your example, hot would be MetaMask, cold would be Paper, and then you would have one or more "other" accounts for "source of funds"? One thing I'm a little dubious about is what kind of account can acquire funds, and call an approve, but can't transfer MKR easily?

I would also like to better understand the trouble you have run into with logs. Unrelated to this project, I have been considering switching to logs for data fetching/aggregation.

[nmushegian]

Here's a clearer way to articulate the solution space: The use pattern enabled right now, without suggestions above, is the only one that allows the cold key to make zero transactions.

For those who can sign transactions with their offline/hardware keys, 90% want direct-voting without a proxy anyway.

Finally, the requirement of separation of control for custody gives its own constraints that are not purely technical. In an approve/pull based system they might have to approve/unapprove/re-limit the proxy as their internal allocations change.