Fix: make operators able to join all specified quorums #268
Conversation
| @@ -75,22 +62,22 @@ func RegisterOperator(ctx context.Context, operator *Operator, transactor core.T | |||
|
|
|||
| privateKeyBytes := []byte(operator.KeyPair.PrivKey.String()) | |||
| salt := [32]byte{} | |||
| copy(salt[:], crypto.Keccak256([]byte("churn"), []byte(time.Now().String()), operator.QuorumIDs[:], privateKeyBytes)) | |||
| copy(salt[:], crypto.Keccak256([]byte("churn"), []byte(time.Now().String()), quorumsToRegister, privateKeyBytes)) | |||
|
|
|||
| // Get the current block number | |||
| expiry := big.NewInt((time.Now().Add(10 * time.Minute)).Unix()) | |||
|
|
|||
| // if we should call the churner, call it | |||
| if shouldCallChurner { | |||
There was a problem hiding this comment.
It looks like there could be another issue here. Suppose an operator has enough stake to enter quorum 0 but not enough for quorum 1, and both quorums are full. Right now, the operator will try to register for both quorums and be rejected by the churner.
We probably need to either:
- Locally simulate the churner check and filter out quorums for which the operator will not be able to join. This is probably the best option, since there's no point in making a churner call if we can tell the operation will fail.
- Update the churner so that it can handle partial failures.
There was a problem hiding this comment.
No. This should be handled in a separate ticket.
|
will close this PR for now to keep the current node cli behavior the same: |
ian-shim
force-pushed
the
operator-register-quorums
branch
from
1d25acf to
6d27ea6
Compare
We're redefining the cli behavior and removing |
|
merge master? |
ian-shim
force-pushed
the
operator-register-quorums
branch
3 times, most recently
from
b71da90 to
821ffe9
Compare
| @@ -75,22 +62,22 @@ func RegisterOperator(ctx context.Context, operator *Operator, transactor core.T | |||
|
|
|||
| privateKeyBytes := []byte(operator.KeyPair.PrivKey.String()) | |||
| salt := [32]byte{} | |||
| copy(salt[:], crypto.Keccak256([]byte("churn"), []byte(time.Now().String()), operator.QuorumIDs[:], privateKeyBytes)) | |||
| copy(salt[:], crypto.Keccak256([]byte("churn"), []byte(time.Now().String()), quorumsToRegister, privateKeyBytes)) | |||
|
|
|||
| // Get the current block number | |||
| expiry := big.NewInt((time.Now().Add(10 * time.Minute)).Unix()) | |||
|
|
|||
| // if we should call the churner, call it | |||
| if shouldCallChurner { | |||
ian-shim
force-pushed
the
operator-register-quorums
branch
from
821ffe9 to
275874d
Compare
Why are these changes needed?
Currently, the operator registration logic will return without opting into specified quorums as long as the operator is registered to at least 1 quorum.
It should make sure the operator is registered to all specified quorums.
This PR also changes the signature of
RegisterOperatormethod to take in churner client instead of taking churner URL. This makes it easier for the method to be tested and allows reusing the client instead of creating a new client every time.Checks