Repository navigation
[guardian-proxy] Forward only committee handoffs the chain stores - #1365
Conversation
|
5336d74 to
e4a0aea
Compare
zhouwfang
left a comment
zhouwfang
left a comment
There was a problem hiding this comment.
The gate only checks each hop's epochs, so a member can stuff a chain with thousands of fake hops that reuse a stored handoff's epochs. The enclave skips them as no-ops, but it still parses every signature first, which gets expensive. Could the gate also check each hop's certificate and committee against the chain?
962e469 to
dfe780b
Compare
502b164 makes a chain consecutive again, so it can use each stored handoff's epochs once, not thousands of times. I'd dropped that rule with the cache in 212eb46. Matching each hop's certificate and committee against the chain isn't in this PR. |
| if known.is_some() { | ||
| return Ok(known); | ||
| } | ||
| let read = tokio::time::timeout(LOOKUP_TIMEOUT, self.source.next_epoch(from_epoch)) |
There was a problem hiding this comment.
Misses aren't cached, so every request with an unstored epoch costs two Sui reads on the same endpoint the allowlist uses. Could we bound misses the way RosterCache does?
There was a problem hiding this comment.
A shared miss budget would let one member starve another's update here. A roster re-read serves every signer, but a lookup is per epoch, so a member spamming unstored epochs would keep taking the budget an honest push needs. I'll bound it per member in a follow-up, since that needs the caller's key in the gate.
There was a problem hiding this comment.
Makes sense, a per-member bound in a follow-up works for me.
| let mut reached = None; | ||
| for transition in transitions { | ||
| let (from_epoch, to_epoch) = epochs(transition).ok_or(Refusal::Malformed)?; | ||
| // A handoff reaches a later epoch, so consecutive ones are distinct |
There was a problem hiding this comment.
This limits hops per request, but a member can still resend the chain as often as they like, and the enclave parses every hop each time. Could we drop hops at or below the guardian's epoch before forwarding?
There was a problem hiding this comment.
That needs the guardian's epoch in the proxy, kept in step with the enclave: GetGuardianInfo is cached for 30s and an update doesn't refresh it. The per-member limit from the other thread caps this too, since a member could then send one chain per interval.
There was a problem hiding this comment.
It doesn't need to be in step. The guardian's epoch only goes up, so a stale value just skips fewer no-op hops. Also, resent chains are all cache hits, so the follow-up would need to limit every update per member, not just misses.
zhouwfang
left a comment
zhouwfang
left a comment
There was a problem hiding this comment.
Approving with the per-member limit as a follow-up.
The two Docker builds failed on Docker Hub's pull rate limit, not on the change. It has cleared now, so could you re-run the failed jobs?
A committee member can push a handoff certificate to the guardian while its reconfig is still pending. If that reconfig then aborts, the guardian is stuck on a committee the chain never activated and every withdrawal stops. The proxy now forwards UpdateCommittee and UpdateCommitteeChain only for handoffs the chain stores, which end_reconfig does: the CommitteeHandoff out of a transition's signing epoch must name its target epoch. A failed Sui read refuses the request.
Stored handoffs never change, so the gate remembers the ones it has read. Replaying the chain's history then costs no Sui reads, and a request at most one lookup that finds nothing. That bound replaces the consecutive rule, which grew with every epoch.
A handoff the chain does not store yet is refused as Unavailable under not_on_chain: its reconfig is pending, or the proxy's fullnode lags. One the chain stored a different handoff over is refused as FailedPrecondition under superseded, which no stale read can produce.
The key-rotation e2e test refused a handoff before any reconfig had started. It now checks the refusal once the reconfig is pending, which is the window the gate exists for.
The handoff key's type address is the Hashi type's only because both are in the original package. The design doc now says the member and handoff checks trust the proxy's Sui fullnode and assume nodes cannot reach the enclave directly.
A chain could repeat one stored handoff any number of times, and the enclave parses every transition's signature before it skips the ones that change nothing. Each handoff must again leave the epoch the one before it reached, so a chain holds a stored handoff at most once.
A separate connection keeps request-driven lookups off the allowlist refresh's, but both still share the endpoint, so "can't hold up" claimed too much.
admit and the module summaries read as if the whole handoff were compared with the chain's. They now say a handoff is matched by its two epochs, and the gate's module doc says its certificate and committee are not compared.
532750d to
f644e14
Compare
2 participants
Summary
A committee member can push a handoff to the guardian while its reconfig can still abort, which leaves the guardian stuck on a committee the chain never activated and stops withdrawals (IOP-792). The proxy now forwards a handoff only once the chain stores it, which
end_reconfigdoes.Changes
node::handoffs::HandoffGateadmits a transition only when the chain stores its handoff.Forwardingruns the gate beforeUpdateCommitteeandUpdateCommitteeChain, and refuses when Sui can't be read.ChainMemberSourcebecomesChainSourceand also reads handoffs.guardian_proxy_handoff_refused_total{reason}metric.Nodes need no change. They already push only stored handoffs and retry a refused push, so a lagging proxy fullnode only delays the update.
The gate matches a handoff's two epochs, not its certificate bytes. An epoch only ever forms one committee, and the enclave still verifies the certificate.