Config: Add explicit observer mode and storage kind #6489
Open
martintomazic wants to merge 5 commits intomasterfrom
Open
Config: Add explicit observer mode and storage kind #6489martintomazic wants to merge 5 commits intomasterfrom
martintomazic wants to merge 5 commits intomasterfrom
Conversation
✅ Deploy Preview for oasisprotocol-oasis-core canceled.
|
martintomazic
commented
Mar 27, 2026
martintomazic
commented
Mar 27, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6489 +/- ##
==========================================
+ Coverage 64.01% 64.09% +0.08%
==========================================
Files 698 698
Lines 68195 68198 +3
==========================================
+ Hits 43655 43713 +58
+ Misses 19499 19460 -39
+ Partials 5041 5025 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
martintomazic
force-pushed
the
martin/breaking/explicit-observer-mode
branch
from
eb8ac90 to
9ba3bd6
Compare
martintomazic
commented
Mar 30, 2026
Comment on lines
+40
to
+42
| if config.GlobalConfig.Consensus.StateKind == consensusCfg.StateKindStateless { | ||
| return nil // Ignore transactions on stateless clients. | ||
| } |
Contributor
Author
There was a problem hiding this comment.
I can bring this in using cfg.accept_gossiped_txs to avoid global state.
The location of this handler is weird though, as this protocol is anyways enabled only for the compute nodes, therefore it doe not fit into common package.
martintomazic
commented
Mar 30, 2026
Comment on lines
+159
to
+164
| // If we have stateless storage, register remote storage. | ||
| // TODO: Client worker should not care about the storage kind. | ||
| // Instead storage initialization should be done during node initialization. | ||
| // Same applies for storage committee worker that initializes and registers local storage, but | ||
| // should intead receive it. | ||
| if config.GlobalConfig.Consensus.StateKind == consensusCfg.StateKindStateless { |
Contributor
Author
There was a problem hiding this comment.
We can do this in two ways:
- Add new
registry.Runtime.GetLocalStoragethat returns local storage if it exits. - Initialize local storage at the node startup and then pass it to common worker (to set it into Runtime container) and to storage worker.
Both options are nice because we can completely decouple storage committee and common worker. Out of scope of this PR, but let's open a separate issue for that...
martintomazic
commented
Apr 1, 2026
martintomazic
commented
Apr 3, 2026
| ) (*Worker, error) { | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| enabled := config.GlobalConfig.Mode.HasLocalStorage() && len(commonWorker.GetRuntimes()) > 0 | ||
| enabled := config.GlobalConfig.HasLocalStorage() && len(commonWorker.GetRuntimes()) > 0 |
Contributor
Author
There was a problem hiding this comment.
Follow-up: This worker shouldn't even be started if no local storage || zero runtimes.
martintomazic
commented
Apr 3, 2026
martintomazic
force-pushed
the
martin/breaking/explicit-observer-mode
branch
3 times, most recently
from
3907d37 to
5edb28b
Compare
kostko
reviewed
Apr 8, 2026
martintomazic
force-pushed
the
martin/breaking/explicit-observer-mode
branch
from
5edb28b to
36458ff
Compare
martintomazic
changed the base branch from
master
to
martin/trivial/observer-e2e-test
martintomazic
force-pushed
the
martin/trivial/observer-e2e-test
branch
from
f09088e to
9b99580
Compare
martintomazic
force-pushed
the
martin/breaking/explicit-observer-mode
branch
from
36458ff to
7904cf5
Compare
martintomazic
commented
Apr 14, 2026
Comment on lines
+20
to
+21
| {name: "keymanager", mode: ModeKeyManager, mustErr: true}, | ||
| {name: "seed", mode: ModeSeed, mustErr: true}, |
Contributor
Author
There was a problem hiding this comment.
Key-manager is technically stateless. Keymanager and seed should probably have separate config, as most of the configs that apply to client/compute are possible (not rejected) but simply ignored.
Out of scope, just this test makes it clearer.
kostko
reviewed
Apr 15, 2026
HasLocalStorage variable was misleading as archive node has the local storage yet the variable was set to false for it. Long term solution is to stop abusing light history for tracking storage worker progress, by possibly following NodeDB last synced round directly.
martintomazic
force-pushed
the
martin/breaking/explicit-observer-mode
branch
from
7904cf5 to
3a8941f
Compare
kostko
approved these changes
Apr 15, 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
2 participants
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.
Closes #6488, follows #6481.