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

api generate invalid tx on Basilisk #6036

Open
xlc opened this issue Nov 22, 2024 · 10 comments
Open

api generate invalid tx on Basilisk #6036

xlc opened this issue Nov 22, 2024 · 10 comments
Labels
P4 - Needs Investigation Requires analysis to determine cause or feasibility. Not fully understood, needs research first.

Comments

@xlc
Copy link
Contributor

xlc commented Nov 22, 2024

I am investigating why this failed: https://github.com/open-web3-stack/polkadot-ecosystem-tests/actions/runs/11964539727/job/33357037908?pr=112

It is generating a signed tx with hex of 0x9902840088dc3417d5058ec4b4503e0c12ea1a0a89be200fe98922423d4334014fa6b0ee0001a4c8b0e0e88d68b8e1a990e3bc0b539d95be71c071e27241e16b9217a27a000b88f67541243c434cb87a1e825a04926d60d286c8da706b3737c2657fa7f108450300009a00000000000080c6a47e8d0300000000000000000004010200411f0100d17c2d7823ebf260fd138f2d7e27d114c0145d968b5ff5006125f2414fadae6900, which pjs apps failed to decode it and runtime rejects it as invalid tx

This is a valid one generated from pjs apps: 0x95028488dc3417d5058ec4b4503e0c12ea1a0a89be200fe98922423d4334014fa6b0ee0001a4c8b0e0e88d68b8e1a990e3bc0b539d95be71c071e27241e16b9217a27a000b88f67541243c434cb87a1e825a04926d60d286c8da706b3737c2657fa7f108450300009a00000000000080c6a47e8d0300000000000000000004010200411f0100d17c2d7823ebf260fd138f2d7e27d114c0145d968b5ff5006125f2414fadae6900

The invalid one have a extra 00 after extrinsic version byte (84). No idea where it is coming from. And it only happens to Basilisk, not other parachains including hydration. Any ideas?

@TarikGul
Copy link
Member

It is generating a signed tx with hex of 0x9902840088dc3417d5058ec4b4503e0c12ea1a0a89be200fe98922423d4334014fa6b0ee0001a4c8b0e0e88d68b8e1a990e3bc0b539d95be71c071e27241e16b9217a27a000b88f67541243c434cb87a1e825a04926d60d286c8da706b3737c2657fa7f108450300009a00000000000080c6a47e8d0300000000000000000004010200411f0100d17c2d7823ebf260fd138f2d7e27d114c0145d968b5ff5006125f2414fadae6900, which pjs apps failed to decode it and runtime rejects it as invalid tx

How is this invalid tx being generated? Any snippet would be super helpful.

@TarikGul TarikGul added the P4 - Needs Investigation Requires analysis to determine cause or feasibility. Not fully understood, needs research first. label Nov 25, 2024
@TarikGul TarikGul moved this from In Review to P4 - Needs Investigation in Polkadot-js general project board Nov 25, 2024
@xlc
Copy link
Contributor Author

xlc commented Nov 25, 2024

to reproduce:

and you should see the bad tx logged from https://github.com/open-web3-stack/polkadot-ecosystem-tests/blob/c963d6f2242dd197afc1f729cb7270f1c3658a00/packages/shared/src/xcm/runXtokenstHorizontal.ts#L45

@bee344
Copy link
Member

bee344 commented Nov 28, 2024

To add to this, the issue becomes evident when the tx is signed via signAsync, since the unsigned gets decoded correctly.

When constructing the same tx through apps, the whole flow works correctly, even when signing without submission. Since the latter also uses signAsync, my guess is that the error is linked to the client returned by setupNetworks, and therefore to chopsticks rather than to the api itself.

@xlc
Copy link
Contributor Author

xlc commented Nov 29, 2024

It was working before and stops working after I upgraded Chopsticks & pjs api version.
However, Hydration shares pretty much the same config and is working fine. All other networks are also fine. This troubles me the most. I am not sure what specific thing in Basilisk triggers this issue.

@bee344
Copy link
Member

bee344 commented Dec 3, 2024

@xlc downgrading @acala-network/chopsticks-testing to 0.16.1 while maintaining the @polkadot/api version has things working fine, so we can confirm that the issue comes from how chopsticks is handling stuff. @acala-network/[email protected] is also breaking, so the breaking change happened between those two versions. Now @0.16.1 chopsticks used @polkadot/[email protected] and then jumped to @polkadot/[email protected]. There were a lot of changes, and big ones, in between those versions, so there's a lot of stuff to check for. My suggestion is to go version to version in chopsticks to see where it breaks and work from there.

@xlc
Copy link
Contributor Author

xlc commented Dec 4, 2024

I am pretty sure it is a bug in pjs because I can't possible think a way for Chopsticks to alter the behavior of signing tx...
If checking diff is too much, how about not checking it?

Here is a minimal reproducible example without Chopsticks:

import { ApiPromise, WsProvider, Keyring } from '@polkadot/api'

const main = async () => {
  const api = await ApiPromise.create({
    provider: new WsProvider('wss://basilisk-rpc.dwellir.com'),
    noInitWarn: true,
  })

  await api.isReady

  const keyring = new Keyring({ type: 'sr25519' })
  const alice = keyring.addFromUri('//Alice')

  const txx = api.tx.xTokens.transfer(1, 10n ** 12n,
    {
      V4: {
        parents: 1,
        interior: {
          X2: [
            { Parachain: 2000 },
            {
              AccountId32: {
                id: '0x0000000000000000000000000000000000000000000000000000000000000000',
              },
            },
          ],
        },
      }
    },
    'Unlimited'
  )
  const signed = await txx.signAsync(alice)
  console.log(signed.toHex())
}

main()

After some debugging, the signer type is GenericMultiAddress, which should be GenericAccountId. So there is a extra byte included. No idea why wrong type is inferred, pjs type registry system is too complicated and I failed to lookup for an answer.

So just need to figure how pjs determine the signer type and see why it is wrong. I suspect there is a possibility that Basilisk is returning some funny metadata.

there is also a warning which maybe related:

2024-12-04 15:09:03 PORTABLEREGISTRY: Unable to determine runtime Call type, cannot inspect sp_runtime::generic::unchecked_extrinsic::UncheckedExtrinsic

@bee344
Copy link
Member

bee344 commented Dec 4, 2024

@xlc you're right, what threw me off was that in apps it's working correctly, which is odd. I've narrowed down the breaking change to being in v13.1.1, so I'll check it out and fix it, I'll keep you posted.

@TarikGul
Copy link
Member

TarikGul commented Dec 4, 2024

The issue seems to be stemming from the use of v15 metadata. In #5941 I made it so the api will default to the use of v15 metadata when possible. So in this case since basilisk has support for the runtime api and v15 it defaults to using v15 metadata. Now, I noticed when I changed the default to be v14 it works fine, but with v15 it sets the account to be a GenericMultiAddress which is what is causing that extra byte.

We haven't narrowed down exactly where in the metadata this issue is happening, or why it's working in apps - but seems like a good start.

@bee344
Copy link
Member

bee344 commented Dec 4, 2024

So apps works because it initializes the api with the types extracted from the typesBunde.ts in apps-config, not with the ones retrieved from the metadata. This issue is likely related to what already happened in sidecar, and there are a couple of PRs and issues from other chains referencing that issue that could be useful to understand what's going on:

In that original issue, such as in this one, the error could be fixed by injecting Basilisk's types bundle directly. A more permanent fix should come from Basilisk's side, since it originates there.

Here's a little example on how to apply the fix:

  const api = await ApiPromise.create({
    provider: new WsProvider('wss://basilisk-rpc.dwellir.com'),
    noInitWarn: true,
    typesBundle: {
        spec: {
            basilisk: {
                "types": [
                    {
                      "minmax": [
                        0,
                        null
                      ],
                      "types": {
                        "AssetPair": {
                          "asset_in": "AssetId",
                          "asset_out": "AssetId"
                        },
                        "Amount": "i128",
                        "AmountOf": "Amount",
                        "Address": "AccountId",
                        "OrmlAccountData": {
                          "free": "Balance",
                          "frozen": "Balance",
                          "reserved": "Balance"
                        },
                        "Fee": {
                          "numerator": "u32",
                          "denominator": "u32"
                        },
                        "BalanceInfo": {
                          "amount": "Balance",
                          "assetId": "AssetId"
                        },
                        "Chain": {
                          "genesisHash": "Vec<u8>",
                          "lastBlockHash": "Vec<u8>"
                        },
                        "Currency": "AssetId",
                        "CurrencyId": "AssetId",
                        "CurrencyIdOf": "AssetId",
                        "Intention": {
                          "who": "AccountId",
                          "asset_sell": "AssetId",
                          "asset_buy": "AssetId",
                          "amount": "Balance",
                          "discount": "bool",
                          "sell_or_buy": "IntentionType"
                        },
                        "IntentionId": "Hash",
                        "IntentionType": {
                          "_enum": [
                            "SELL",
                            "BUY"
                          ]
                        },
                        "LookupSource": "AccountId",
                        "Price": "Balance",
                        "ClassId": "u64",
                        "TokenId": "u64",
                        "ClassData": {
                          "is_pool": "bool"
                        },
                        "TokenData": {
                          "locked": "bool"
                        },
                        "ClassInfo": {
                          "metadata": "Vec<u8>",
                          "total_issuance": "TokenId",
                          "owner": "AccountId",
                          "data": "ClassData"
                        },
                        "TokenInfo": {
                          "metadata": "Vec<u8>",
                          "owner": "AccountId",
                          "data": "TokenData"
                        },
                        "ClassInfoOf": "ClassInfo",
                        "TokenInfoOf": "TokenInfo",
                        "ClassIdOf": "ClassId",
                        "TokenIdOf": "TokenId",
                        "OrderedSet": "Vec<AssetId>",
                        "VestingSchedule": {
                          "start": "BlockNumber",
                          "period": "BlockNumber",
                          "period_count": "u32",
                          "per_period": "Compact<Balance>"
                        },
                        "VestingScheduleOf": "VestingSchedule",
                        "LBPWeight": "u32",
                        "WeightCurveType": {
                          "_enum": [
                            "Linear"
                          ]
                        },
                        "PoolId": "AccountId",
                        "BalanceOf": "Balance",
                        "AssetType": {
                          "_enum": {
                            "Token": "Null",
                            "PoolShare": "(AssetId,AssetId)"
                          }
                        },
                        "Pool": {
                          "owner": "AccountId",
                          "start": "BlockNumber",
                          "end": "BlockNumber",
                          "assets": "AssetPair",
                          "initial_weights": "LBPWeight",
                          "final_weights": "LBPWeight",
                          "weight_curve": "WeightCurveType",
                          "pausable": "bool",
                          "paused": "bool",
                          "fee": "Fee",
                          "fee_receiver": "AccountId"
                        },
                        "AssetDetails": {
                          "name": "Vec<u8>",
                          "asset_type": "AssetType",
                          "existential_deposit": "Balance",
                          "locked": "bool"
                        },
                        "AssetDetailsT": "AssetDetails",
                        "AssetMetadata": {
                          "symbol": "Vec<u8>",
                          "decimals": "u8"
                        },
                        "AssetInstance": "AssetInstanceV1",
                        "MultiLocation": "MultiLocationV1",
                        "MultiAsset": "MultiAssetV1",
                        "Xcm": "XcmV1",
                        "XcmOrder": "XcmOrderV1"
                      }
                    }
                  ]
            }
        }
    }
  })

Let us know if you have any more questions

@xlc
Copy link
Contributor Author

xlc commented Dec 4, 2024

So we need to know why metadata v14 and v15 are giving out different types (or if they are in fact giving out different types). I will ask Hydration team to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 - Needs Investigation Requires analysis to determine cause or feasibility. Not fully understood, needs research first.
Projects
Status: P4 - Needs Investigation
Development

No branches or pull requests

3 participants