feat(bridge-v1): add safe harbour address with admin controls and RPC#83
Merged
Conversation
barakshani
reviewed
Apr 24, 2026
Collaborator
barakshani
left a comment
There was a problem hiding this comment.
overall looks good, left some small comments. But why is this only for defcon 1? we can activate the address both on defcon 1 and 3, the difference is just time delay until they are activated.
86cdfc8 to
221a9a5
Compare
221a9a5 to
a5572e0
Compare
|
Commit: b039cf8
|
Lets clients fetch the safe harbour address from the bridge state at a given Bitcoin block hash, completing the read path for the new state field added earlier in this branch.
The bridge subprotocol init config now requires a `safe_harbour_address` descriptor. Without it the runner panics on params load and every functional test fails the ASM-ready probe.
storopoli
approved these changes
May 19, 2026
Member
storopoli
left a comment
There was a problem hiding this comment.
ACK 421a26e
Tests are really lacking: There is no test asserting initial deactivated state, address updates, activation on both Defcon messages, or RPC serialization.
Given this is consensus state plus admin-control behavior, I’d add focused unit tests around BridgeV1Subproto::process_msgs and one RPC/harness check before merging.
The safe harbour ships as consensus state and is surfaced by the `getSafeHarbour` RPC, so its activation semantics and JSON shape are load-bearing for both the protocol and clients. Add unit coverage for the initial deactivated state, the activation flag, address updates preserving activation, and SSZ + JSON serde round-trips.
`process_msgs` is the only entry point through which the admin subprotocol can mutate the safe harbour, so its behaviour for each inter-protocol message variant is consensus-critical. Pin down that `UpdateSafeHarbourAddress` swaps the address without activating, that both Defcon1 and Defcon3 flip the activation flag, and that an admin address update after activation keeps the new address active.
Cover the new `strata_asm_getSafeHarbour` endpoint end-to-end against a live runner: assert that without any admin defcon signal the configured address surfaces verbatim, that the payload is deactivated, and that the response is stable across processed blocks. The RPC is the only client-facing surface of the safe harbour, so its JSON shape is part of the contract clients will rely on.
MdTeach
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a safe harbour address to the Bridge V1 subprotocol. The address is configured at init time (in
BridgeV1InitConfig) and held in the bridge state as deactivated. The bridge reacts to inter-protocol messages from the admin subprotocol to update the destination descriptor and to activate the safe harbour on Defcon signals. A new RPC method exposes the current value to clients.Type of Change
Note to Reviewers
The full flow will be tested once we add the
StrataSecurityCouncilrole as part of #81.Checklist
Related Issues
STR-3181