-
Notifications
You must be signed in to change notification settings - Fork 64
Added support for renew and index host function
#2189
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
Conversation
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>
lib/src/executor/host/functions.rs
Outdated
| HostFunction::ext_hashing_twox_256_version_1 => { | ||
| crate::signature!((vm::ValueType::I64) => vm::ValueType::I32) | ||
| } | ||
| HostFunction::ext_transaction_index_index_version_1 => { |
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.
@x3c41a please also at the end
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
|
@x3c41a By the time this gets merged, can we just build/install it locally (somehow) from this branch and use it in our JS/PAPI examples? |
InstallingYes, so I've built local JS package and linked it: Then I went to bulletin repo and linked newly build (and linked) local smoldot repo with: To make sure you did everything right, just run the following in bulletin repo and the result should be similar to: TestingOnce I made sure smoldot is linked to the build from my new branch, I started testing. and my transaction were timing out on That's my findings for now, I'll update the thread if I find anything interesting. |
|
@x3c41a very nice, so it looks like that this HostFunctions fix is working and we are one step further, so we need to solve other error When I grep @x3c41a At least, what we could do, is to add some logs to the smoldot, to see what protocol is missing. I am not that expert here, but maybe @dmitry-markin or @lexnv could know? When I was debugging Bulletin logs with And also, we are at least adding some grandpa protocol for solochain: Most probably, the smoldot does not support Bitswap (yet?), because our Bulletin nodes start also Bitswap protocol streams, not sure if this could be an issue:
|
wow, good to hear 🚀 |
Looks like it helped! Smoldot started sending responses back: |
|
Writes work too ✅ I think we're good to go now! |
|
mhm, I found another issue while implementing authorise and store with IPFS and PAPI - paritytech/polkadot-bulletin-chain#118 TL;DR: the content that we store with Smoldot + Bulletin can't be fetched back via IPFS |
That is the most probably not smoldot issue, let's focus here on merging these host functions and continue with other stuff in Bulletin PR/issue |
I am also unsure if it's smoldot issue or not thus decided to raise awareness before the PR is merged |
tomaka
left a comment
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.
The no-op implementation is unfortunately problematic, because a Polkadot node might refuse a block when one of the two new functions is called with an invalid extrinsic index. However for the sake of pragmatism I think it's fine to merge anyway.
|
@tomaka how does release process look like from smol-dot side? what's the ETA for this change to go live? Thank you! |
|
New versions have already been published. |
Added no-op implementation for both.
Relates to #2182