serviceability: halt and resume a feed - #4307
Conversation
`FeedStatus` and the gate that reads it landed with the subscriber work, but nothing could move the status. A staked feed sat in `Pending` for life and no feed had a way to stop publishing. Two instructions and the transitions they refuse. Halt goes only from `Active`, resume only from `Halted`, and each refusal says which way it is wrong rather than sharing one error. `Pending` to `Active` is deliberately absent: that step re-reads the stake mirror, so it belongs with the code that does, and leaving it out here keeps the two apart. The feed's own builder may sign either one, which no other feed instruction allows. RFC-28 makes halt the builder's lever and doubles it as a way to rotate the upstream source without redeploying, so a builder that cannot halt its own feed cannot do either. A `FEED_AUTHORITY` or `FOUNDATION` key may sign as well, because a feed whose builder has gone quiet must still be stoppable.
There was a problem hiding this comment.
🟢 Approval recommended
The on-chain lifecycle transitions, authorization rules, instruction wiring, error mappings, and new integration tests are consistent and complete for the stated D1 scope.
Pull request overview
This pull request adds feed lifecycle controls to the DoubleZero serviceability on-chain program so an authorized signer can halt an Active feed and resume a Halted feed, unblocking status transitions now that FeedStatus gating exists.
Changes:
- Add
HaltFeed(variant 120) andResumeFeed(variant 121) instruction variants, entrypoint routing, and processors that enforce the transition table viaFeedNotHaltable(124) andFeedNotResumable(125). - Add a shared
require_feed_writerauthorization helper that allows either the feedbuilderor aFEED_AUTHORITY | FOUNDATIONsigner to perform lifecycle changes. - Add a new SVM test suite covering allowed transitions and refusal cases, including authorization-path differentiation.
File summaries
| File | Description |
|---|---|
| smartcontract/programs/doublezero-serviceability/src/processors/feed/halt.rs | New processor to transition Active -> Halted with explicit refusal error otherwise. |
| smartcontract/programs/doublezero-serviceability/src/processors/feed/resume.rs | New processor to transition Halted -> Active with explicit refusal error otherwise. |
| smartcontract/programs/doublezero-serviceability/src/processors/feed/mod.rs | Expose halt/resume modules and add require_feed_writer to support builder-or-permission authorization. |
| smartcontract/programs/doublezero-serviceability/src/instructions.rs | Add instruction variants 120/121 plus unpack/name/debug wiring. |
| smartcontract/programs/doublezero-serviceability/src/entrypoint.rs | Route HaltFeed / ResumeFeed to the new processors. |
| smartcontract/programs/doublezero-serviceability/src/error.rs | Add FeedNotHaltable/FeedNotResumable and map them to custom codes 124/125. |
| smartcontract/programs/doublezero-serviceability/tests/feed_lifecycle_test.rs | New integration tests for lifecycle transitions and refusal/authorization behavior. |
| crates/doublezero-serviceability-instruction/src/feed.rs | Add instruction builders for halt_feed and resume_feed. |
| CHANGELOG.md | Document the new instructions, transition constraints, and signer rules. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
armcconnell
left a comment
armcconnell
left a comment
There was a problem hiding this comment.
Checked against edge-feed-spec GLOSSARY.md (a5f5cdf): conforms.
See inline comments.
|
|
||
| let mut feed = Feed::try_from(feed_account)?; | ||
| let globalstate = GlobalState::try_from(globalstate_account)?; | ||
| require_feed_writer( |
There was a problem hiding this comment.
[Important] Resume is gated on the same key set as halt, so a builder can immediately undo a FEED_AUTHORITY/FOUNDATION halt; with Retired unreachable and DeleteFeed refusing staked feeds, nothing then stops a live staked feed.
There was a problem hiding this comment.
You are right, and this was a hole rather than a nit. Fixed in d0a6b09c9.
I had given the builder co-equal power on resume, which meant an operator's halt bought nothing: the builder reverses it in the next transaction. And as you say, there is no second lever. Retired is unreachable until D2, and DeleteFeed refuses a staked feed. So the halt was the only way to stop a staked feed, and it did not stop one.
Feed now carries halted_by, set to the signer that halted:
- Halted by anyone other than the feed's builder: only a
FEED_AUTHORITYorFOUNDATIONkey can resume. - Halted by the builder: either the builder or an operator can resume, which keeps the source-rotation workflow the instruction exists for.
A pubkey rather than a flag, so an operator can see who did it, and so D2 has it. It appends to the account and reads as zero on a feed written before the field, which is right: such a feed cannot have been halted.
Two tests, and neither passes without the change: a builder is refused with NotAllowed on an operator's halt and the status stays Halted, and the builder lifts its own halt successfully.
| /// `builder` may sign this one, so a builder can rotate its upstream source without an operator. | ||
| /// A `FEED_AUTHORITY` or `FOUNDATION` key can sign it too, which is why this stays on the | ||
| /// permission-appending path. | ||
| pub fn halt_feed(program_id: &Pubkey, payer: &Pubkey, feed: &Pubkey) -> Instruction { |
There was a problem hiding this comment.
[Minor] halt_feed and resume_feed have no test; test_feed_pubkey_verbs pins the tag byte and account metas for update and delete but not for 120/121.
There was a problem hiding this comment.
Fixed in d0a6b09c9. test_feed_pubkey_verbs now pins 120 and 121 alongside 113 and 114, tag byte and account metas both.
Worth saying why that test earns its keep here specifically: DoubleZeroInstruction::unpack matches the leading byte by hand and ends in a catch-all, so a wrong or missing tag compiles cleanly and arrives at the program as InvalidInstructionData. That is exactly how it failed the first time I ran these tests, and the tag assertion is what would have caught it at the crate boundary instead.
| return Err(DoubleZeroError::FeedNotResumable.into()); | ||
| } | ||
|
|
||
| feed.status = FeedStatus::Active; |
There was a problem hiding this comment.
[Minor] Resume re-enters Active without re-reading the StakeMirror — the check Pending to Active is deferred to G1 for. Once G1 lands, a staked feed can halt, have its mirror corrected downward, and resume uncovered.
There was a problem hiding this comment.
Taken, and closed now rather than left for G1. Fixed in d0a6b09c9.
Resume re-reads the mirror and re-proves the cover, through a new require_stake_still_covers. It is not verify_stake_covers_rate: creation claims an unspent stake and so requires feed_key to be empty, while here the feed already holds the claim, so the mirror must name this feed and no other. What both check is the tier.
A staked feed that omits its mirror account is refused with StakeMirrorMissing rather than read as having no stake to check. That was the shape of the original bug, so it gets its own test.
I did not defer this. You are right that it is unreachable today, since a staked feed cannot reach Active and therefore cannot be halted. But G1 is sprint 3, and a known hole that waits for the feature that opens it is how holes ship. Closing it now costs one helper and two tests.
The test corrects the mirror down to StakeTier::None while the feed sits halted, then asserts resume is refused with StakeDoesNotCoverRate.
Three review comments, and the first one is a hole. Resume took the same signers as halt, so a builder undid an operator's halt the moment it landed. `Retired` is unreachable until D2 and `DeleteFeed` refuses a staked feed, so that halt is the only lever an operator has, and a builder that can reverse it leaves no lever at all. `Feed` now records `halted_by`, and a halt by anyone other than the builder takes an operator to lift. A builder's own halt is still the builder's to lift, which is the source rotation the whole instruction exists for. Resume also re-proves the stake. A mirror can be corrected downward while a feed sits halted, so resuming on the check made at creation would let a feed publish at a rate its stake no longer backs. A staked feed that omits its mirror is refused rather than read as having no stake to check. The builders had no test. They now pin the tag byte and the account metas, which is what catches a wrong variant: `unpack` matches the leading byte by hand with a catch-all, so the compiler never sees it.
Implements D1.
FeedStatusand the gate that reads it landed with the subscriber work, but nothing could move the status: a staked feed sat inPendingfor life, and no feed had a way to stop publishing.The transition table
HaltFeedResumeFeedPendingActiveHaltedHaltedActiveRetiredEach refusal says which way it is wrong,
FeedNotHaltable(124) orFeedNotResumable(125), rather than sharing one error. A caller that halts twice should learn it did not stop something a second time, not read a generic argument failure.PendingtoActiveis deliberately absent. That step re-reads the stake mirror, which is the only thing that catches a mirror corrected after it admitted a feed, so it belongs with the code that does it (G1). Leaving it out here keeps the two apart rather than half-implementing it.Who can sign
The feed's own
buildermay sign either instruction. No other feed instruction consultsfeed.builderat all; they authorize onFEED_AUTHORITY | FOUNDATIONand stop there.Both paths are needed, for different reasons:
FEED_AUTHORITYorFOUNDATIONkey, because a feed whose builder has gone quiet must still be stoppable, and every other feed instruction already works that way.A catalog feed has no builder, so only the second path applies to it. The check compares against the default pubkey explicitly rather than relying on nobody being able to sign as it.
Testing Verification
Five tests in a new
feed_lifecycle_test.rs. A new file rather than helpers bolted ontofeed_test.rs, because itsinit_stakedhardcodes a builder the test cannot sign as, and other tests depend on that.Pendingfeed neither halts nor resumes. This also guards G1's territory: nothing here moves a feed out ofPending.Pendingfeed can be halted by anyone and G1 does not exist yet to make a staked feedActive. The errors are what distinguish them: the builder reaches the status check and getsFeedNotHaltable, the stranger does not get past authorization and getsNotAllowed.That last one is the honest shape of the test available today. A staked feed cannot reach
Activeuntil G1, so the builder's successful halt cannot be tested yet, and asserting on which check stopped the call is the strongest available evidence that the authorization path works.Full suite passes: 327 serviceability tests plus the 5 new ones, 86 in the instruction crate,
clippy --all-targets -Dwarningsandfmt --checkclean.One thing the compiler could not catch, worth knowing for the next variant:
DoubleZeroInstruction::unpackmatches the leading byte by hand with a catch-all, andFrom<ProgramError> for DoubleZeroErrormatches the code the same way. Both compile fine with a variant missing. The first showed up asInvalidInstructionDataat runtime and the second as a round-trip test failure.