Skip to content
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

feat!: speicfy on-chain parameters explicitly in genesis #4812

Merged
merged 3 commits into from
Jul 8, 2024

Conversation

Erigara
Copy link
Contributor

@Erigara Erigara commented Jul 8, 2024

Description

Currently it's not possible to submit genesis which doesn't pass under default parameter values (i.e. large genesis).
This PR fixes that allowing to specify parameters inside genesis.json so that they are applied before processing rest of instructions.

@Erigara Erigara self-assigned this Jul 8, 2024
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Jul 8, 2024
genesis/src/lib.rs Outdated Show resolved Hide resolved
configs/swarm/genesis.json Outdated Show resolved Hide resolved
@mversic
Copy link
Contributor

mversic commented Jul 8, 2024

the downside of this approach is that SetParameter instruction will not get validated with the executor specified in the genesis.json

@Erigara
Copy link
Contributor Author

Erigara commented Jul 8, 2024

I see, another downside is that custom parameters is not registered yet.
In this case we can left Upgrade to be first instruction, then we should make sure that set parameters works fine under new executor default parameters.

@Erigara Erigara force-pushed the explicit_genesis_params branch from 6bde1a2 to 54b6739 Compare July 8, 2024 14:12
@mversic
Copy link
Contributor

mversic commented Jul 8, 2024

I see, another downside is that custom parameters is not registered yet.
In this case we can left Upgrade to be first instruction, then we should make sure that set parameters works fine under new executor default parameters.

We can make Upgrade be the first instruction but not check it for executor limits. Then apply parameters and then all other

@Erigara
Copy link
Contributor Author

Erigara commented Jul 8, 2024

I see, another downside is that custom parameters is not registered yet.
In this case we can left Upgrade to be first instruction, then we should make sure that set parameters works fine under new executor default parameters.

We can make Upgrade be the first instruction but not check it for executor limits. Then apply parameters and then all other

Regestring executor itself is not that heavy.
So i don't expect any problems under default parameters.
Executor limits is a problem when we validate transaction with a lot of instructions in it.

@mversic
Copy link
Contributor

mversic commented Jul 8, 2024

Also it's possible to set parameters on executor migration. But I like the simplicity of providing them through genesis.json

@s8sato s8sato self-assigned this Jul 8, 2024
data_model/src/block.rs Show resolved Hide resolved
@Erigara Erigara merged commit dc6a0c6 into hyperledger-iroha:main Jul 8, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants