Threshold Enforcement (revised) #33
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements "Threshold Enforcement (revised)" - a significant refactoring that introduces key set management and updates the threshold enforcement mechanism. The changes include:
- Core architectural changes: Refactored from a single root key list to a key set-based architecture with
KeySetConfigsupporting multiple key sets, each with their own configuration including minimum thresholds, monetary values, and realm associations - Dependency updates: Upgraded critical dependencies (scc 2→3.3.2, sdd 3→4.2.4, ethabi 16→18, blsful 3.0.0-pre8→3.0.0, data-encoding 2.8→2.9)
- API enhancements: Added new v1 handshake endpoint that returns key set information, updated contract ABIs to support key set identifiers in routing data
Reviewed changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/lit-os/Cargo.lock | Consolidated duplicate dependency versions (bitvec, ethabi, ethereum-types, etc.) |
| rust/lit-node/lit-node/Cargo.toml | Added log dependency, updated scc version |
| rust/lit-node/lit-sdk/src/handshake.rs | Renamed types from Json prefix to plain names (JsonSDKHandshakeRequest → SDKHandshakeRequest) |
| rust/lit-node/lit-sdk/src/cait_sith.rs | Added missing import for k256 |
| rust/lit-node/lit-node/src/models/mod.rs | Added KeySetConfig and PubKeyRoutingData model structures |
| rust/lit-node/lit-node/src/config/chain.rs | Refactored from single root_keys list to key_sets HashMap with KeySetConfig |
| rust/lit-node/lit-node/src/tss/common/curve_state.rs | Complete rewrite to support key set-based root key retrieval |
| rust/lit-node/lit-node/src/tss/common/traits/signable.rs | Changed signature to pass key_set_id instead of root_pubkeys |
| rust/lit-node/lit-node/src/tss/dkg/manager.rs | Updated to handle multiple key sets during epoch changes |
| rust/lit-node/lit-node/src/tss/dkg/engine.rs | Added ShadowOptions struct, improved shadow realm key handling |
| rust/lit-node/lit-node/src/endpoints/web_client.rs | Added handshake v1 endpoint, refactored v0 for backwards compatibility |
| rust/lit-node/lit-node/tests/integration/*.rs | Updated test fixtures to include key_set_id in PKP info tuples |
| rust/lit-node/lit-node/tests/component/*.rs | Updated component tests to use new DKG APIs with key set identifiers |
| rust/lit-core/lit-blockchain/src/contracts/*.rs | Updated contract bindings for new ABI with keySetIdentifier fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function checkValidatorCountAgainstKeySetsInRealm( | ||
| uint256 realmId, | ||
| uint256 validatorCnt, | ||
| uint256 reason | ||
| ) internal view { | ||
| bytes32[] memory keySetIds = s().keySetIds; | ||
| for (uint256 i = 0; i < keySetIds.length; i++) { | ||
| LibStakingStorage.KeySetConfig memory config = s().keySetsConfigs[ | ||
| keySetIds[i] | ||
| ]; | ||
| for (uint256 j = 0; j < config.realms.length; j++) { | ||
| if (config.realms[i] == realmId) { | ||
| if (validatorCnt < config.minimumThreshold) { | ||
| if (reason == 1) { | ||
| revert("Not enough validators for key set"); | ||
| } else if (reason == 2) { | ||
| revert NotEnoughValidatorsInNextEpoch( | ||
| validatorCnt, | ||
| config.minimumThreshold | ||
| ); | ||
| } else if (reason == 3) { | ||
| revert StakingValidatorFacet | ||
| .CannotKickBelowKeySetThreshold( | ||
| config.identifier | ||
| ); | ||
| } | ||
| } | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
In checkValidatorCountAgainstKeySetsInRealm, the inner loop indexes config.realms with i instead of j, so it only checks (or even reverts on) a single realm index per key set rather than iterating over all associated realms. This mis-indexing can cause the minimum validator threshold for some key sets and realms to be skipped entirely, allowing operations (like validator kicks or realm migrations) that drop the validator count below the configured minimumThreshold for those key sets. Update the condition to use config.realms[j] == realmId so that every realm associated with a key set is correctly considered when enforcing validator-count thresholds.
WHAT
Threshold enforcement across keyset ( realm ) boundaries.
Replace #12 which required refactoring of tests for size related reasons.
Most of the "changed size" of this PR is just moving the
registerAttestedWalletfunction to a new facet.