polkadot-api / polkadot-api

Polkadot-API is a meticulously crafted suite of libraries, each designed to be composable, modular, and aligned with a "light-client first" philosophy
https://papi.how
MIT License
93 stars 22 forks source link

Error when executing `Utility.batch_all` #505

Closed polkadotbeefy closed 5 months ago

polkadotbeefy commented 5 months ago

When executing Utility.batch_all, I'm getting a console error:

Uncaught (in promise) Error: Incompatible runtime entry Tx(Utility.batch_all)

Code:

    const client = createClient(getSmProvider(assethubChain))
    const api = client.getTypedApi(assetHub)

    const transfers = receivers.map(({ address, amount }) =>
      api.tx.Assets.transfer_keep_alive({
        id: ASSET_ID,
        amount: BigInt(amount),
        target: MultiAddress.Id(address),
      }).decodedCall
    )

    const batchCall = api.tx.Utility.batch_all({ calls: transfers })
    batchCall.signAndSubmit(account.polkadotSigner)
      .then(console.log)

I'm using the latest versions:

  "dependencies": {
    "@polkadot-api/descriptors": "latest",
    "polkadot-api": "latest",

When executing the transactions one-by-one without batching, I don't experience any issues:

    api.tx.Assets.transfer_keep_alive({
      id: ASSET_ID,
      amount: BigInt(amount),
      target: MultiAddress.Id(recipientAddress),
    })
      .signAndSubmit(account.polkadotSigner)
      .then(console.log)

Any idea what could be the issue here?

Here's the full code: https://github.com/polkadotbeefy/polkadot-asset-hub-test-papi/blob/feature/batch-transactions/src/App.tsx

josepot commented 5 months ago

First and foremost, thanks a lot for taking the time to report this issue and for providing all this context.

Any idea what could be the issue here?

Yes, I know exactly what the issue is.

TL; DR: Running pnpm papi update && pnpm papi from the root folder should update your descriptors and the issue should be gone.

That being said, I really am not happy with the fact that Utility.batch* throws the Error: Incompatible runtime entry whenever there is a runtime change that changes one of the transactions.

We need to come up with a better solution. However, in the meanwhile, we should have special logic for certain things like Utility.batch*.

This is what's happening:

Every time that polkadot-api generates the types/descriptors, it also creates a checksum for each one of the chain interactions so that later we can check during runtime whether the chain interaction that your dApp is about to perform is in fact compatible with the on-chain-runtime of your network.

In other words, the encoders/decoders that are used in your dApp are generated dynamically "on the fly". So, it would be risky to just pass to assume that the interaction exist or that it's the same as it was when the types were generated. Therefore, what we do is we have a checksum that lets us know if a particular interaction has changed.

However, any little change will produce a different checksum. This is super inconvenient in cases like Utility.batch because its checksum depends on all the checksums of all the possible calls.

Fortunately, we just had a team brainstorming meeting and we came up with a very promising idea that will solve this issue. Also, @carlosala is going to work on a proper technical write-up to explain this solution.

In a nutshell, what we are going to be doing in the future is: rather than computing the checksum of the chain interaction and comparing it with the checksum that was previously computed when the descriptors were generated. What we will do is to check whether the whole interaction (with its input) is compatible with interaction that would have taken place with the runtime that you had when the descriptors were generated. We will create a proper issue explaining this solution and its challenges.

In the meanwhile, we will probably add some especial logic for some common interactions like Utility.batch*, etc. So, that the current shortcomings are less problematic.

polkadotbeefy commented 5 months ago

@josepot thanks a lot for the speedy reply! Highly appreciated.

TL; DR: Running pnpm papi update && pnpm papi from the root folder should update your descriptors and the issue should be gone.

Even though I ran pnpm papi update && pnpm papi and restarted the dev build (npm run dev), it failed with the same error as previously, unfortunately. Is there anything else I can try? Did you have the capacity to test my code on your machine, and if so, did it work?

However, any little change will produce a different checksum. This is super inconvenient in cases like Utility.batch because its checksum depends on all the checksums of all the possible calls.

Oh, so since we can combine all kind of transactions within a batch call, there is basically an unlimited amount of possible checksums?! Not sure if I fully understood - to me it seems in both the current and future scenario you'd compare the checkum of the current interaction with the previously generated one (whenever papi is run, I guess).

Anyway, good job so far on this library, it's very well-readable. I'm confident it will become easier to use once it matures and docs are updated :)

LMK if you need further data, e.g. if you want me to run some other commands and share the output. Thx!

josepot commented 5 months ago

Did you have the capacity to test my code on your machine, and if so, did it work?

Yep:

https://github.com/polkadot-api/polkadot-api/assets/8620144/edf4223d-6b8d-4dbd-afe8-9b456e5bdd78

Oh, so since we can combine all kind of transactions within a batch call, there is basically an unlimited amount of possible checksums?! Not sure if I fully understood - to me it seems in both the current and future scenario you'd compare the checkum of the current interaction with the previously generated one (whenever papi is run, I guess).

I did a horrible job at explaining myself, but long-story short: this won't be a problem anymore in the future... However, as of right now, if you are using utility.batch (et al) then your codegen must be up-to-date with the latest metadata version that triggered changes on any of the transactions. However, we will tackle this very soon, so that if the transactions that you are using are still compatible, that you don't get this error. We will work on a proper technical write-up explaining the tradeoffs of each solution and what we are going to do to make things better.

Anyway, good job so far on this library, it's very well-readable. I'm confident it will become easier to use once it matures and docs are updated :)

Thanks a lot!

LMK if you need further data, e.g. if you want me to run some other commands and share the output. Thx!

I don't know what to tell you... I did try the code and everything seems to be working just fine after running pnpm papi update && pnpm papi, which is why I'm not sharing the video above. Please let me know if there is something that I missed. Thanks!

PS: try running rm -rf ./node_modules/.vite in order to clear the vite cache, just in case... Or perhaps try with: git clean -dfx && pnpm i && pnpm papi update && pnpm papi && pnpm dev

polkadotbeefy commented 5 months ago

@josepot thanks a lot for elaborating and taking the time to test. I emptied the cache, ran papi again and after that it worked!

Great stuff, closing this!

ryanleecode commented 1 month ago

I just encountered this error. Has there been any additional headway with this?

josepot commented 1 month ago

I just encountered this error. Has there been any additional headway with this?

well, yeah, the new compatibility API completely avoids these issues.