-
Notifications
You must be signed in to change notification settings - Fork 92
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
[WIP] Add support for Moonriver #95
Conversation
@xlc is there an easy way to troubleshoot the block produced, including changes in the storage (maybe debug logs level ?)? |
Line 25 in 4386ee5
with trace level logging you will see storage changes chopsticks/src/blockchain/txpool.ts Line 130 in 4386ee5
|
Currently; it is possible to create 1 block with extrinsic (transfer) but fails for the following block:
|
@crystalin I was able to produce new blocks for moonriver and did 2 trasfers but I had to delete randomness.notfirstblock before and after block creation to bypass randomness validation.
Add this before
Add this after
|
@crystalin also I used your config.yml but without block number |
@ermalkaleci thanks, it seems to work for me too. Howerver it only support Runtime 2000 (which wasn't the one used initially but Moonriver has been upgraded since). Concerning the hard coded changes, do you have an idea how to include them better ? |
because smoldot job is to sync with current state of chains, it doesn't support old tx version. regarding hard coded changes I am not sure, maybe it can be inside consensus closure. I didn't spend much time, just tried to cheat, not sure if we can modify digest logs to make it valid |
@ermalkaleci Thank you. I also tested a runtime upgrade and it worked, but it can't produce blocks anymore after that (same issue I believe):
|
@crystalin can you try refreshing polkadotjs app after the upgrade? upgrade subscription isn't implemented yet |
I did, as otherwise I can't send transaction. It showed the new version and supported to send the transaction but chopsticks failed. |
Here is a RT2100 for Moonriver. You need to "enactAuthorizedUpgrade" (I added the authorization in the genesis of moonriver.yml): https://drive.google.com/file/d/1vVVR-YosRspnUEIHg9iLmI4nrBBffEXy/view?usp=sharing |
@crystalin ok I think I know the reason. We're not updating metadata when upgrade happens. Fixing it |
@crystalin fixed |
@ermalkaleci I merged master and still get the same issue |
Did you rebuilt wasm? |
Yes I did (otherwise it doesn't even restart) |
Ok so I think it might be due to a transaction that stays in the pool during the runtime upgrade. and then fails to be applied at the following block. |
You can go to js section and do api.rpc("dev_newBlock") |
I tested your runtime. Upgrade was successfully but I got the error trying to build new blocks. Is this runtime compatible with smoldot? |
You are right, the runtime might have been broken. I didn't really check with commit I built it from. |
You may want to add tests like this to avoid accidentally breaks the compatibility https://github.com/AcalaNetwork/chopsticks/blob/master/e2e/relaychain.test.ts |
src/blockchain/txpool.ts
Outdated
@@ -97,6 +112,7 @@ export class TxPool { | |||
}, | |||
}) | |||
|
|||
head.pushStorageLayer().set(compactHex(meta.query.randomness.notFirstBlock()), StorageValueKind.Deleted) |
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.
@xlc not sure what we could do here. Ideally, the signature should be mocked too but quickly looking at the randomness pallet I don't see much how this could be done.
A temporary solution is to remove the NotFirstBlock but it needs to be pallet specific and also break support for randomness.
Any idea ?
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.
how does the randomness pallet verify the VRF? if that's using host function than we can mock it. otherwise I can see few options:
- modify randomness pallet to make it mock friendly. e.g. if a particular storage exists or not exists, switch to mock mode
- refactor chopsticks to support plugins so we can better manage pallet specific code
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.
So it checks the NotFirstBlock there https://github.com/PureStake/moonbeam/blob/master/pallets/randomness/src/lib.rs#L294
And the verification is (few lines under) pointing to https://github.com/PureStake/moonbeam/blob/master/pallets/randomness/src/vrf.rs#L32
@ermalkaleci latest master seems broken (I tried to update submodule run yarn and ....)
|
yeah forgot ts-node registrer-ts-paths. fixing |
Co-authored-by: Ermal Kaleci <[email protected]>
@ermalkaleci: Done, it seems to work well with current code |
Is this ready for merge @crystalin ? |
@xlc it is yes |
I merged master but is now failing the test :( |
Status: Working, missing better fix for Randomness pallet VRF
It adds:
Broken: