Update 0030-ETHM-multisig_control_spec.md

[C0deMunk33]

No description provided.

[davidsiska-vega]

I don't think we want to insist on per-epoch updates.

The validators will have protocol-level incentives to update the multisig weights if they weights or the signer set changes.

[C0deMunk33]

I'll change the wording of it and call it something else

[C0deMunk33]

This has been updated in the spec and the code

[emilbayes]

I think we should consider the signature format a bit more, both taking into context what Clef supports and also additional safe-guards.

I propose the following format:

concat(
  bytes2("\x19\x00"), bytes20(address(this)), // [1]
  u256(block.chainid), // [2]
  bytes32(current_signer_set_hash), // [3]
  bytes20(calling_contract), // [4]
  u256(nonce), // [5]
  bytes(function_payload) // [6]
)

1. This prefix is to accommodate Clef's text/validator mime-type. See their source
2. This prevents replay across chains for whatever reason. It's unlikely that the same signer set and multisig address will be present across chains, but this rules out that completely
3. This is the signer set discussed in the PR. I would also be beneficial if the signer set had a total-order such that when verifying signatures, the verification algorithm can be O(n) in the signers and not have to keep a set of seen signers like the current implementation. The order has to be easy to compute on the Vega side, so a simple lexicographic sort of the validator keys would be very easy. Or it could sort by weight in descending order. This discussion is a bit orthogonal as it touches in the verification of multiple signatures rather than the integrity of a particular signature.
4. Making the calling_contract explicit. In the current format we have the concept of submitter which has a dual-use; when the multisig contact is called by another contract, this becomes the address of the calling contact, however when it is the multisig contract verifying a call to itself, it becomes the address of whoever invokes the contact. In this case we should inject address(this) from the multisig contract itself and keep msg.sender for external contracts. Again, this discussion is more in the realm of the mechanics of how the multisig is used rather than the signature itself. The multisig contract doesn't have a concept of other contracts, but can simply look at the current signer set and whether a quorum of those signed identical payloads.
5. The nonce is a common-denominator control to prevent replay attacks since we cannot guarantee the order in which transactions to the multisig contract are done. This might as well be a part of the function_payload, however I do think it makes sense to keep a explicit nonce rather than a dummy signer as proposed below such that we do not need any special case logic around verifying signatures against the signer set, eg. stop the verification one short of the signer set, since the last signer is a dummy and someone might just be able to produce a signature with a key that hashes to an address that overlaps with the dummy.
6. The function name and parameters as describe in the PR.

[C0deMunk33]

[1] why do we care about clef? Is it giving us anything useful for the extra gas?
[2] might consider doing this as it would allow validators to reuse addresses across multiple chains if we get there... A) they probably shouldn't do that and B) need to investigate the extra gas
[3] that hash is entirely derived from the passed in signer set rather than storing signers, this also includes the weights (good call and already implemented, will update the spec to make that clear)
[4] at the moment I have an internal validation function and an external one (see source) Does this work?
[5] how common/feasable is sig recovery overlap for ETH addresses? (looking it up right after I post this)
[6] cool

[emilbayes]

1. So in Vega you can either import an ethereum key or use a running clef service. Currently we have a fork of clef to support the current signature format: https://docs.vega.xyz/testnet/node-operators/how-to/rotate-ethereum-keys
2. Cost here is low. CHAINID is 2 gas, memory expansion and hashing additional bytes is 6 and 3 gas respectively.
3. Correct, my point was to define a total order, but as mentioned, it's something that should be defined elsewhere in the spec
4. Main point here is today, if you submit anything to the multisig contract itself, you need to use a fixed external account, while other contracts such as the bridge, can be submitted by anyone, since the "point of view" of who the submitter is changes. What I'm suggesting is that we make this the same so when transactions to the multisig contract itself are validated, we use the multisig address as the "submitter"
5. I don't remember what the point was here
6. 👍

[C0deMunk33]

1. I'll need to investigate clef more
2. seems reasonable, added to the spec on the end of the "final hash"
3. this was added to the spec as one of the assumptions
4. submitter is now always the contract calling the verification and never a user. even if it's on the multisig itself, this allows anyone to submit any of the multisig orders and removes that complication

[C0deMunk33]

adding the clef prefix after a chat with @jeremyletang

[emilbayes]

I had an informal chat with Jeremy and then David on Friday because we were all in the same place, and the following points came up:

1. Migration
   Since we change the call signature for verifying function calls, we need to consider how
   we migrate the bridge (should be the easy one) but also the the asset pool (the harder one).
   Perhaps a single large withdrawal authorised on the bridge to a new pool will do, but of
   course we need to do this in a manner that will make the community confident that the assets
   are withdrawn directly to a new pool and not a externally controlled address.
2. Sorting pubkeys and signatures
   I mentioned this point in my comments on what we need to sign and how we sign in a way that
   has broader support (some validators use clef). My proposal is to sort validator addresses
   lexicographically, and that signatures to be sorted by the validator they map to. This
   lets us not have to encode empty signatures like eg gravity bridge, but we do the same as
   bitcoin where the process is (pseudo code):

threshold = 6667 # 2/3 threshold, probably an input to the function
validator_ptr = 0 # index of last validator we verified
cumulative_weight = 0 # how much weight have we accumulated
for sig in signatures:
  address = ecrecover(message, signature)
  for j in range(validator_ptr, length(validators)): # slice from the last validator we verified
    if address == validator[j]:
      cumulative_weight =+ weights[j]
      if cumulative_weight >= threshold: return True # success
      validator_ptr = j
return False # we iterated over all signatures and a validators without reaching threshold

3. Bridge using asset pool multisig
   One minor "expensive" call right now in the bridge is that it calls the asset pool to ask
   what the current multisig contract registered there is. This is an additional transaction
   costing in the ballpark of 10k gas (and probably a bit less but that magnitude). I know
   Danny has previously mentioned that Klaus had some confusion attack or something around this
   but it would be good to clear up exactly if there is an attack and write a comment about it.
   Otherwise it will shave off a fair bit of gas if we can inline it into the bridge contract.
   The specific function is multisig_control_address() on the bridge.
4. Updating the validator set on each epoch
   This one makes all of use feel a bit uneasy in the sense that it will require validators to
   be quite proactive and invalidate a withdrawals right after the epoch boundary and until
   the validators have submitted the epoch update with new weights and validator set. Withdrawals
   are invalidate because of the "dummy" validator address. However David mentioned an incentive
   mechanism on the Vega side, where rewards are paid as the minimum of min(vega_weight, eth_weight).
   Of course the eth_weight is not directly readable, but can be inferred by looking at the hash on
   the ethereum side and tracking the mapping from hash to validator set on Vega. If there is
   economic gain from updating the validator set on the ethereum side, validators would do so,
   but if there are only small fluctuations in their total stake, perhaps they would just keep the
   set as is. It will also let withdrawals stay valid for longer in the best case, and invalidate them in the same cadence as currently proposed, if weights (or the set of validators) change enough for there to be an economic gain.

[wwestgarth]

4. Updating the validator set on each epoch

I had similar feelings about this. I wrote a little script that looked at the changes in stake scores of each validator on mainnet over each epoch, and there are some big movements, but mostly there there are just tiny changes and I don't know what security benefit we get from updating to weights are that practically equal to some small tolerance. We also currently have this mechanism in core where we penalise a validator if they aren't on the contract but should be, and penalise all validators if someone is on who shouldn't be, and maybe that doesn't really fit anymore with the weights changes.

Also on this:
"Of course the eth_weight is not directly readable, but can be inferred by looking at the hash on
the ethereum side and tracking the mapping from hash to validator set on Vega."

It makes me wonder what the course of action for the protocol is if the set_hash emitted from ethereum is different that one Vega expects it to be (i.e it has been changed externally for some reason, maybe intentionally by manually gather validator signatures for some good reason). Vega-core won't be able to know anything about whats happened from just the hash.

[C0deMunk33]

@emilbayes

1. migration: there's a migration v1->v2 plan in the spec which includes a proposal for swapping out the bridge logic with a migration contract that can only migrate funds to the new asset pool
2. Sorting pubkeys and signatures: given that the entire set needs to be hashed in order to match on-chain, we cannot re-sort per tx. Maybe I am missing your point here?
3. Bridge using asset pool multisig: this was specifically recommended by the last audit IIRC as it doesn't enforce syncing between the contract. This means there is a chance that 1 contract uses a different multisig than the other
4. Updating the validator set on each epoch: there is no enforcement for updating each epoch, they can be updated whenever the network feels its appropriate. You are correct that we would not be able to see the weights on-chain, but to issue multisig orders, vega MUST know the validators and weights to get the hash to match.
   As for
   "what the course of action for the protocol is if the set_hash emitted from ethereum is different that one Vega expects it to be" it would be possible to store the addresses+weights on-chain with each update as bytes just for the sake of being able to see, but reaching into memory for each verification is more expensive (and exponential) than passing them in as calldata. (cc: @wwestgarth)