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

nimbus author inherent mock #648

Merged
merged 3 commits into from
Jan 27, 2024
Merged

nimbus author inherent mock #648

merged 3 commits into from
Jan 27, 2024

Conversation

ermalkaleci
Copy link
Contributor

No description provided.

@ermalkaleci ermalkaleci mentioned this pull request Jan 26, 2024
@ermalkaleci ermalkaleci merged commit 509c624 into master Jan 27, 2024
6 checks passed
@ermalkaleci ermalkaleci deleted the nimbus-author-inherent branch January 27, 2024 08:25
@ermalkaleci
Copy link
Contributor Author

@girazoki Does your system require authoritiesNoting.setLatestAuthoritiesData to be called on every block? This change will allow your chain to build blocks but I don't know if all functionalities will work. Can you help with testing?

@girazoki
Copy link
Contributor

Yes I can definitely! Yes we need that to be set in all the blocks for now.

BTW I spoke to parity about being able to proof all keys that you want inside set-validation-data. This way we can just take in chopsticks whatever comes from validation-data in the previous block and modify only the keys corresponding to XCM,etc (so similar to what you are doing right now)

We need a bit of time for this, but hopefully we can propose the PR to parity in the next weeks.

What can I help you with? Do you want me to try a specific version?

@ermalkaleci
Copy link
Contributor Author

@girazoki test latest beta 0.9.8-2 and let me know if it works

@girazoki
Copy link
Contributor

girazoki commented Jan 30, 2024

@ermalkaleci we have not been able to use directly the version you point at:

npx @acala-network/chopsticks@0.9.8-2 --help throws:

const _jsondiffpatch = require("jsondiffpatch");
                       ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /home/girazoki/Desktop/tanssi/node_modules/.pnpm/jsondiffpatch@0.6.0/node_modules/jsondiffpatch/lib/index.js from /home/girazoki/Desktop/tanssi/node_modules/.pnpm/@acala-network+chopsticks@0.9.8-2_debug@4.3.4/node_modules/@acala-network/chopsticks/dist/cjs/utils/decoder.js not supported.
Instead change the require of index.js in /home/girazoki/Desktop/tanssi/node_modules/.pnpm/@acala-network+chopsticks@0.9.8-2_debug@4.3.4/node_modules/@acala-network/chopsticks/dist/cjs/utils/decoder.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/home/girazoki/Desktop/tanssi/node_modules/.pnpm/@acala-network+chopsticks@0.9.8-2_debug@4.3.4/node_modules/@acala-network/chopsticks/dist/cjs/utils/decoder.js:12:24) {
  code: 'ERR_REQUIRE_ESM'
}

However after hacking a bit in the version branch we have been able to make it work and we confirm that is able to fork correctly our networks. We might need to propose one additional inherent to make it fully compatible but we can do this, as right now it seems much easier.

Thanks!

@ermalkaleci
Copy link
Contributor Author

@girazoki I've pushed a new beta with the fix

@girazoki
Copy link
Contributor

girazoki commented Jan 30, 2024

I confirm it works with our container-chains, thank you so much! As I said for the tanssi chain we need an additional inherent, but we should be able to do a PR for it. Will let you know when we have it!

Once again, thank you!

@ermalkaleci
Copy link
Contributor Author

Yes, with the refactored inherent it should be straightforward

@girazoki
Copy link
Contributor

girazoki commented Feb 1, 2024

#665 CC: @ermalkaleci

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants