-
Notifications
You must be signed in to change notification settings - Fork 276
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
dynamic parameter value #4352
Comments
Relevant here #4028. We should consider that there might be arbitrary parameters used by executor and we have hard-coded parameters used by wasm engine, sumeragi. Maybe we can define all on-chain parameters which look like smt like this:
What do you think @mversic @0x009922? I should notice that executor already can kinda have arbitrary parameters by either using |
That's a good point. Are you saying that we can just reuse |
what's the problem here? |
It's not exactly a problem, but it would be nice if crucial on-chain parameters would be strongly typed with exact types (not stored as random JSON strings) |
Yes, it can place them in any metadata, since permissions for the executor aren't checked. |
I don't mind any format for executor parameters as long as WASM/Sumeragi parameters are strictly specified and typed. |
that's internally, no? Externally (from the client side), we should do it in the same way, i.e. as JSON string? |
Not sure, i think if we expect for |
if performance is a concern, I expect updating a parameter to be really a rare operation. If the fallibility is concern, I your operation across network can fail for miriad of reasons. I'm trying to formulate the simplest API, and not have specialized API for different edge cases unless really needed. It also complicates the code base |
if we store parameters in metadata then we would have two different ways to update parameters |
For context, what I think hard-coded configuration should look like, in a form of structures in the data model: struct ConfigurationState {
max_transactions_in_block: U32,
block_time_ms: u64,
commit_time_ms: u64,
// metadata limits, wasm limits...
}
struct ConfigurationStatePartial {
max_transactions_in_block: Option<u32>,
block_time_ms: Option<u64>,
commit_time_ms: Option<u64>,
// ...
}
enum InstructionBox {
// ...
Configure(ConfigureBox)
}
struct ConfigureBox(ConfigurationStatePartial); And also to patch the pub enum ConfigurationEvent {
Changed(ParameterId),
Created(ParameterId),
Deleted(ParameterId),
} to pub enum ConfigurationEvent {
Changed(ConfigurationState),
} As for current parameters, I don't understand what is going to happen to them yet. It seems we need to clearly distinguish "static parameters" from "dynamic, custom parameters" (maybe call it "executor parameters" explicitly?). Both sets are a part of "chain wide configuration". What are your guys opinion on the data model changes I would like to introduce, and on how we can distinguish these two sets of parameters? |
I agree about struct for hard-coded configuration. |
Assigned to myself, as I see it as a natural part of #4028. |
Numeric
is sufficient because values should be loosely typed and checked in executorGiven the previous 2 points, I think that instead of
ParameterValueBox
we could just haveStringWithJson
as we do for permission tokens. This is because the value for parameters defined in executor can indeed be anything. Even though there are some hard coded, I don't think it's necessarily worth to have them strongly typed inside an enumThis is something I think should be explored in a separate PR
Originally posted by @mversic in #4305 (comment)
The text was updated successfully, but these errors were encountered: