Repository navigation
[WNS-56] OPP: bound the operator roster to the schedule window and cap queueout #692
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
Changes from all commits
0e82d29
fa823e2
29d1187
19d3596
8594a73
01314ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -250,22 +250,21 @@ void epoch::setconfig(uint32_t epoch_duration_sec, | ||||||||||||||||
| check(operators_per_epoch > 0, "operators_per_epoch must be positive"); | |||||||||||||||||
| check(batch_op_groups > 0, "batch_op_groups must be positive"); | |||||||||||||||||
| check(batch_op_groups <= MAX_BATCH_OP_GROUPS, | |||||||||||||||||
| "batch_op_groups exceeds the uint8 group-index ceiling (255)"); | |||||||||||||||||
| "batch_op_groups exceeds the outpost group ceiling (8)"); | |||||||||||||||||
| // Widen the product to uint64 so a large operators_per_epoch cannot wrap the | |||||||||||||||||
| // uint32 multiply into a small value that spuriously matches a small | |||||||||||||||||
| // batch_operator_minimum_active; an honest product beyond uint32 simply fails | |||||||||||||||||
| // the equality against the uint32 field, which is the intended rejection. | |||||||||||||||||
| check(static_cast<uint64_t>(operators_per_epoch) * batch_op_groups == batch_operator_minimum_active, | |||||||||||||||||
| "batch_operator_minimum_active must equal operators_per_epoch * batch_op_groups"); | |||||||||||||||||
| // Magnitude bounds, checked after the relationship above so an inconsistent | |||||||||||||||||
| // Magnitude bound, checked after the relationship above so an inconsistent | |||||||||||||||||
| // triple reports the equality error. The equality alone cannot reject an | |||||||||||||||||
| // internally consistent but absurd size (UINT32_MAX groups of one), which | |||||||||||||||||
| // would later abort advance() on its schedule-window vector reserves and | |||||||||||||||||
| // halt epoch advancement chain-wide. | |||||||||||||||||
| // internally consistent but absurd size (UINT32_MAX operators in one group), | |||||||||||||||||
| // which would later abort advance() on its schedule-window vector reserves and | |||||||||||||||||
| // halt epoch advancement chain-wide. With both factors bounded, the window | |||||||||||||||||
| // cannot exceed MAX_SCHEDULED_BATCH_OPERATORS. | |||||||||||||||||
| check(operators_per_epoch <= MAX_OPERATORS_PER_EPOCH, | |||||||||||||||||
| "operators_per_epoch exceeds the per-epoch schedule ceiling (100)"); | |||||||||||||||||
| check(batch_operator_minimum_active <= MAX_SCHEDULED_BATCH_OPERATORS, | |||||||||||||||||
| "batch_operator_minimum_active exceeds the schedule-window ceiling (1000)"); | |||||||||||||||||
| "operators_per_epoch exceeds the outpost group-size ceiling (7)"); | |||||||||||||||||
| check(epoch_retention_envelope_log_count > 0, | |||||||||||||||||
| "epoch_retention_envelope_log_count must be positive"); | |||||||||||||||||
|
|
|||||||||||||||||
|
|
@@ -735,7 +734,11 @@ void epoch::finishadv(uint32_t epoch_index, int64_t emission_amount) { | ||||||||||||||||
|
|
|||||||||||||||||
| state_tbl.set(state, ram_payer); | |||||||||||||||||
|
|
|||||||||||||||||
| // Queue OPERATORS attestation (full roster with authex chain addresses) for each outpost. | |||||||||||||||||
| // Queue the OPERATORS roster for each outpost: every operator seated in the window, with its current | |||||||||||||||||
| // status and authex chain addresses. Outposts read the roster only to map the window's members to | |||||||||||||||||
| // the keys they deliver with (Solana also refuses a window that seats an ineligible member), so the | |||||||||||||||||
| // window is all they need, and the roster stays within MAX_SCHEDULED_BATCH_OPERATORS however many | |||||||||||||||||
| // operators register. A member whose opreg row was pruned is left out. | |||||||||||||||||
| // IMPORTANT: Must come before BATCH_OPERATOR_GROUPS so that the ETH outpost's | |||||||||||||||||
| // _handleOperators populates operatorEthAddress before _handleBatchOperatorGroups | |||||||||||||||||
| // looks up those addresses. | |||||||||||||||||
|
|
@@ -745,61 +748,70 @@ void epoch::finishadv(uint32_t epoch_index, int64_t emission_amount) { | ||||||||||||||||
| authex::links_t authex_links(AUTHEX_ACCOUNT); | |||||||||||||||||
| auto links_by_name = authex_links.get_index<"byname"_n>(); | |||||||||||||||||
|
|
|||||||||||||||||
| for (auto it = opreg_ops.begin(); it != opreg_ops.end(); ++it) { | |||||||||||||||||
| opp::attestations::OperatorEntry entry; | |||||||||||||||||
| entry.account.name = it->account.to_string(); | |||||||||||||||||
| entry.type = it->type; | |||||||||||||||||
| entry.status = it->status; | |||||||||||||||||
|
|
|||||||||||||||||
| // Collect all authex-linked chain addresses for this operator. | |||||||||||||||||
| // Store raw public key bytes from the variant (33 bytes for EM/secp256k1, | |||||||||||||||||
| // 32 bytes for ED/Ed25519). | |||||||||||||||||
| auto link_it = links_by_name.lower_bound(it->account.value); | |||||||||||||||||
| while (link_it != links_by_name.end() && link_it->username == it->account) { | |||||||||||||||||
| opp::types::ChainAddress chain_addr; | |||||||||||||||||
| chain_addr.kind = link_it->chain_kind; | |||||||||||||||||
|
|
|||||||||||||||||
| std::visit([&](const auto& key_data) { | |||||||||||||||||
| using T = std::decay_t<decltype(key_data)>; | |||||||||||||||||
| if constexpr (std::is_same_v<T, webauthn_public_key>) { | |||||||||||||||||
| // EM (secp256k1 compressed) — 33 bytes in key.key | |||||||||||||||||
| chain_addr.address.assign(key_data.key.begin(), key_data.key.end()); | |||||||||||||||||
| } else if constexpr (std::is_same_v<T, ed_public_key>) { | |||||||||||||||||
| // ED (Ed25519) — 32 bytes | |||||||||||||||||
| chain_addr.address.assign( | |||||||||||||||||
| reinterpret_cast<const char*>(key_data.data()), | |||||||||||||||||
| reinterpret_cast<const char*>(key_data.data() + key_data.size())); | |||||||||||||||||
| } else if constexpr (std::is_same_v<T, ecc_public_key>) { | |||||||||||||||||
| // K1/R1 (secp256k1/P-256 compressed) — 33 bytes | |||||||||||||||||
| chain_addr.address.assign(key_data.begin(), key_data.end()); | |||||||||||||||||
| } | |||||||||||||||||
| // Skip BLS keys — not used for chain address linking | |||||||||||||||||
| }, link_it->pub_key); | |||||||||||||||||
| for (const auto& group : state.batch_op_groups) { | |||||||||||||||||
| for (const auto& member : group) { | |||||||||||||||||
| const auto op = opreg_ops.try_get(opreg::operator_key{member.value}); | |||||||||||||||||
| if (!op) continue; | |||||||||||||||||
|
|
|||||||||||||||||
| opp::attestations::OperatorEntry entry; | |||||||||||||||||
| entry.account.name = member.to_string(); | |||||||||||||||||
| entry.type = op->type; | |||||||||||||||||
| entry.status = op->status; | |||||||||||||||||
|
|
|||||||||||||||||
| // Collect all authex-linked chain addresses for this operator. | |||||||||||||||||
| // Store raw public key bytes from the variant (33 bytes for EM/secp256k1, | |||||||||||||||||
| // 32 bytes for ED/Ed25519). | |||||||||||||||||
| auto link_it = links_by_name.lower_bound(member.value); | |||||||||||||||||
| while (link_it != links_by_name.end() && link_it->username == member) { | |||||||||||||||||
| opp::types::ChainAddress chain_addr; | |||||||||||||||||
| chain_addr.kind = link_it->chain_kind; | |||||||||||||||||
|
|
|||||||||||||||||
| std::visit([&](const auto& key_data) { | |||||||||||||||||
| using T = std::decay_t<decltype(key_data)>; | |||||||||||||||||
| if constexpr (std::is_same_v<T, webauthn_public_key>) { | |||||||||||||||||
| // EM (secp256k1 compressed) — 33 bytes in key.key | |||||||||||||||||
| chain_addr.address.assign(key_data.key.begin(), key_data.key.end()); | |||||||||||||||||
| } else if constexpr (std::is_same_v<T, ed_public_key>) { | |||||||||||||||||
| // ED (Ed25519) — 32 bytes | |||||||||||||||||
| chain_addr.address.assign( | |||||||||||||||||
| reinterpret_cast<const char*>(key_data.data()), | |||||||||||||||||
| reinterpret_cast<const char*>(key_data.data() + key_data.size())); | |||||||||||||||||
| } else if constexpr (std::is_same_v<T, ecc_public_key>) { | |||||||||||||||||
| // K1/R1 (secp256k1/P-256 compressed) — 33 bytes | |||||||||||||||||
| chain_addr.address.assign(key_data.begin(), key_data.end()); | |||||||||||||||||
| } | |||||||||||||||||
| // Skip BLS keys — not used for chain address linking | |||||||||||||||||
| }, link_it->pub_key); | |||||||||||||||||
|
|
|||||||||||||||||
| entry.addresses.push_back(std::move(chain_addr)); | |||||||||||||||||
| ++link_it; | |||||||||||||||||
| } | |||||||||||||||||
|
|
|||||||||||||||||
| entry.addresses.push_back(std::move(chain_addr)); | |||||||||||||||||
| ++link_it; | |||||||||||||||||
| ops_attest.operators.push_back(std::move(entry)); | |||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High — cap or chunk the roster to what Ethereum can dispatch atomically. The new limits permit 8 groups × 16 members, while
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lowered, as you suggest:
The 8 × 7 case will be a permanent test in wire-ethereum's |
|||||||||||||||||
| } | |||||||||||||||||
|
|
|||||||||||||||||
| ops_attest.operators.push_back(std::move(entry)); | |||||||||||||||||
| } | |||||||||||||||||
|
|
|||||||||||||||||
| std::vector<char> encoded; | |||||||||||||||||
| auto out = zpp::bits::out{encoded, zpp::bits::no_size{}}; | |||||||||||||||||
| (void)out(ops_attest); | |||||||||||||||||
| // An empty roster tells an outpost nothing (Solana skips it as an anomaly), so an empty window | |||||||||||||||||
| // queues none; the BatchOperatorGroups withhold below reports that state. | |||||||||||||||||
| if (!ops_attest.operators.empty()) { | |||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High — an empty post-slide window preserves removed operators’ outpost authority. This new guard suppresses the authoritative OPERATORS snapshot when the window becomes empty. On the same transition, finishadv also withholds BATCH_OPERATOR_GROUPS when the next group is empty (lines 891–939), so neither outpost receives a revocation. This is reachable with the allowed 1-group/1-operator configuration. The sole non-bootstrap operator can deliver the current epoch and then request its full collateral withdrawal; opreg::withdraw immediately re-evaluates eligibility. The next slide removes that now-non-ACTIVE member and selects an empty tail. Previously, the full roster would have carried UNKNOWN/SLASHED/TERMINATED; this branch emits neither control attestation. An initialized Solana outpost retains the old ACTIVE mapping and active group. Ethereum only clears an omitted/inactive resident seat when its OPERATORS handler runs, and its retained one-slot anchor can authorize that old address for the next epoch. With a 1×1 quorum, the removed key can submit a forged next envelope containing OPERATORS and BATCH_OPERATOR_GROUPS entries that re-seat itself. Please provide an explicit fail-closed revocation shape that both outposts apply (for example bounded terminal tombstones or coordinated authoritative-empty support), or stop advancement when no authoritative next group can be conveyed. Add an initialized-outpost regression covering a nonempty→empty transition and assert that the old key cannot submit the next epoch. The new opreg slash comment is also still inaccurate on this path because the claimed group update is withheld.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I built the tombstone variant to test this: every roster listed the expired group's departing members with their real status. I also wrote initialized-outpost regressions for it on Ethereum and Solana. It doesn't close the attack, so I've left it out and squashed the PR.
What happens to the old key after N:
Closing this needs changes outside the roster, tracked in WIRE-423:
The slash comment no longer claims the groups update. It now says rosters list the operator as SLASHED while it is seated and leave it out once its group has left. |
|||||||||||||||||
| std::vector<char> encoded; | |||||||||||||||||
| auto out = zpp::bits::out{encoded, zpp::bits::no_size{}}; | |||||||||||||||||
| (void)out(ops_attest); | |||||||||||||||||
|
|
|||||||||||||||||
| sysio::chains::chains_t chains_tbl(CHAINS_ACCOUNT); | |||||||||||||||||
| for (auto it = chains_tbl.begin(); it != chains_tbl.end(); ++it) { | |||||||||||||||||
| if (!is_active_outpost(*it)) continue; | |||||||||||||||||
| action( | |||||||||||||||||
| permission_level{get_self(), "owner"_n}, | |||||||||||||||||
| MSGCH_ACCOUNT, | |||||||||||||||||
| "queueout"_n, | |||||||||||||||||
| std::make_tuple( | |||||||||||||||||
| it->code.value, | |||||||||||||||||
| opp::types::ATTESTATION_TYPE_OPERATORS, | |||||||||||||||||
| encoded | |||||||||||||||||
| ) | |||||||||||||||||
| ).send(); | |||||||||||||||||
| sysio::chains::chains_t chains_tbl(CHAINS_ACCOUNT); | |||||||||||||||||
| for (auto it = chains_tbl.begin(); it != chains_tbl.end(); ++it) { | |||||||||||||||||
| if (!is_active_outpost(*it)) continue; | |||||||||||||||||
| action( | |||||||||||||||||
| permission_level{get_self(), "owner"_n}, | |||||||||||||||||
| MSGCH_ACCOUNT, | |||||||||||||||||
| "queueout"_n, | |||||||||||||||||
| std::make_tuple( | |||||||||||||||||
| it->code.value, | |||||||||||||||||
| opp::types::ATTESTATION_TYPE_OPERATORS, | |||||||||||||||||
| encoded | |||||||||||||||||
| ) | |||||||||||||||||
| ).send(); | |||||||||||||||||
| } | |||||||||||||||||
| } | |||||||||||||||||
| } | |||||||||||||||||
|
|
|||||||||||||||||
|
|
|||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Medium — keep cluster-tool schedule validation in sync with these limits.
wire-tools-tsstill validates withMAX_BATCH_OP_GROUPS = 255,MAX_OPERATORS_PER_EPOCH = 100, andMAX_SCHEDULED_BATCH_OPERATORS = 1000inBatchOperatorSchedule.ts. Configurations such as 9 groups of 1 or one group of 17 therefore pass tool preflight and fail later inepoch::setconfigduring cluster startup. Please update those constants and their boundary tests in a companion change, ideally from a shared or generated source of truth, so invalid schedules are rejected before startup.