-
Notifications
You must be signed in to change notification settings - Fork 246
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
remove pallet-history-seeding and all related stuff #3303
base: main
Are you sure you want to change the base?
Conversation
This is a tricky change, because we need to know the encoding of the call to extract the data from it. If we’d used But since we used a custom history seeding call, I think we’ll need to keep a I don’t know if the weights can be deleted, is it ok to set the weight of a call that always fails to zero? (Or an extremely large number?) Or will that cause trouble with past transaction validation, or future invalid transaction handling? Another possibility is to delete the crate entirely, and use the binary encoding of the call. But I think we’d need to modify the parser to do that, because once we delete the history seeder, those calls will be parsed as unknown or invalid. Maybe other people on the team can think of other alternatives? |
@nazar-pc do you have an opinion here? |
No, we do not. The mapping is determined by the runtime, specifically mapping for a particular block is dictated by the code in parent block's runtime and on the client side transactions are just opaque blobs. Runtime API that client calls will not change if we change the set of pallets, that is the whole point of the runtime API as such. So we can safely remove the pallet and mapping will continue working properly for both old and new blocks as long as runtime API doesn't change or client is updated to support both versions if we do have to change it at some point in the future. |
Is there a way to convert this feature to a devnet-only version? The |
If your only goal is to populate the network with data, you can use |
@teor2345 can you take this over? From what I read above we only need to revert the change to object mappings in the first commit and test whether they still work as expected. |
My understanding is that this change is correct as is. Previously seeded history will be mapped correctly by the current version of the runtime. Future history seeds won’t happen, so there’s no need for the next version of the runtime to support them. (And no practical way to test without deploying the current version of the runtime, then upgrading it to a version with this change.) |
As discussed on Protocol sync, this needs runtime version increase before merge. |
@teor2345 can you bump the runtime spec version and get this in ? |
Removing history seeding from runtime as planned all along: we don't want to have the risk of uploading data for free.
However, this seems to break @teor2345 functionality to get object mapping of the data that was already seeded at genesis if the runtime call is gone. Keeping this draft until we can align with Teor on how to address that (if it's indeed an issue).
Code contributor checklist: