-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Node service plugins #5056
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
Node service plugins #5056
Conversation
| let mut file = std::fs::File::create(&operator_path)?; | ||
| // Script that reads stdin and writes it to stdout. | ||
| writeln!(file, "#!/bin/sh")?; | ||
| writeln!(file, "cat")?; |
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.
this is cool
| // The counter app doesn't implement the task processor interface (nextActions, etc.), | ||
| // so the task processor will fail to query it. But we've verified that: | ||
| // 1. The CLI arguments are accepted. | ||
| // 2. The node service starts successfully. | ||
| // 3. The task processor is initialized (even if it can't query the app). | ||
|
|
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.
meh :)
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.
I (or rather Claude) extended the test.
| //! | ||
| //! The task processor watches specified applications for requests to execute off-chain tasks, | ||
| //! runs external operator binaries, and submits the results back to the chain. |
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.
This design adds an integration flow that goes onchain app -> offchain logic – this is a second one alongside the oracle calls that we have.
I think this is wrong as it's creating hard-to-change flows: if we want to add a new trigger, we'll need to update the app. Which is more difficult in "Linera world" as all instances of the app would have to be migrated as well.
Why not use oracle calls? Or events? One uses events to recreate onchain state of the app and that's all you need to trigger the offchain action.
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.
I'm not sure how oracle calls could replace this flow: The goal of this is to also support data sources that aren't available (e.g. aren't HTTP-whitelisted) for the validators, and are provided by whoever is running the app backend.
Events could probably be used: that would add slightly more data to he block body that could be considered redundant, since the only consumer of the event is the backend that is running the full chain anyway, and they are not meant to be subscribed to by any other chains.
The current approach with the service query also has the advantage that the service doesn't have to remember anything after a restart: it can just make another query to the app to find out which actions are upcoming. If we were using events, we would have to keep track of which ones were already processed?
Anyway, I don't have a strong opinion; I simply used the same approach as the original PR.
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.
I'm not sure how oracle calls could replace this flow:
I didn't mean this particular flow but in general - oracles are for onchain logic to request validators to do sth.
Events could probably be used: that would add slightly more data to he block body that could be considered redundant, since the only consumer of the event is the backend that is running the full chain anyway, and they are not meant to be subscribed to by any other chains.
Events are used to publish state changes. Offchain logic can reconstruct the state and act on it. This should suffice here.
if we were using events, we would have to keep track of which ones were already processed?
In a sense, yes, by the virtue of reconstructed state. But a "node service" for the app has to follow the chain/app anyway in order to propose blocks on it so it would have the events anyway.
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.
As far as I understand, this does not request the validators to do sth, it requests the proposer to do sth. So it seems that using a different way is quite appropriate that way.
An alternative I can see is to do all this in an external process, separate from the node service, that monitors some chains and then does stuff like proposing blocks / following chains based on what it sees, but the node service is essentially the part of the system that is supposed to react to blocks, so making it extensible like this makes sense to me.
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.
I am not advocating against the system of extensions. What I'm simply saying is that we shouldn't be adding new mechanism for onchain apps to signal to offchain processes to do sth. We already have two:
- oracles
- events
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.
But it's not really a new mechanism, is it? It's just a third one in your list, that we already have anyway:
- service queries
linera-service/src/cli/main.rs
Outdated
| // Start the task processor if operator applications are specified. | ||
| if !operator_application_ids.is_empty() { | ||
| let operators = Arc::new(operators.into_iter().collect()); | ||
| info!("Supported operators: {:?}", operators); |
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.
| info!("Supported operators: {:?}", operators); | |
| info!(?operators, "supported operators"); |
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.
Not sure: INFO-level logs are user-visible. Maybe we shouldn't even use {:?}, and join the operators instead?
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.
Done in 6fcbd8d.
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.
I'm not convinced. We (I think) agreed to some format of the log and we shouldn't change it. Now you create a precedence for the author to decide how the log line should look like and we will end up discussing them again. This is a node service code, not a 3rd party (external) app.
|
Closing as superseded by #5072 |
Motivation
On a private repository Mathieu started working on an alternative node service with a plugin system for external processes that want to interact with apps and commit operations.
Proposal
Add the plugin system to the node service itself. (All done by Claude so far.)
Test Plan
CI
(Claude added a test; to extend that we'd have to write an example app that requests actions from a plugin.)
Release Plan
testnet_conway, thenLinks