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

Update signer payload known types to include TAssetConversion for assetId #6081

Closed
wants to merge 4 commits into from

Conversation

Chralt98
Copy link

@Chralt98 Chralt98 commented Jan 28, 2025

Fixes #6074

TAssetConversion: 'Option<MultiLocation>'

In the above TAssetConversion is Option<MultiLocation>, but should assetId be Option<TAssetConversion> like here? In this case we would have Option<Option<MultiLocation>> in the end, right?

@TarikGul
Copy link
Member

TarikGul commented Jan 28, 2025

So AFAIU, the reason TAssetConversion is wrapped in an Option is because it's an optional input for the Extrinsic class in PJS. In addition to that the Option type that the MultiLocation is wrapped in is exactly what the chain expects, therefore it needs to be wrapped in an option when converted to it byte value.

^ I need to double check the above as well though.

@Chralt98
Copy link
Author

What I find confusing though, is the metadataHash example.

It uses an Option too here. But in the test, the metadataHash doesn't change through calling toPayload from here to here. So, why should the hex value of TAssetConversion change?

@Chralt98
Copy link
Author

I mean this override alone will replace the knownTypes including TAssetConversion. So, why not using the default override TAssetConversion to be just MultiLocation instead of Option<MultiLocation>?

@Chralt98
Copy link
Author

Chralt98 commented Jan 29, 2025

Ok, with the latest commit I get: Bad input data provided to validate_transaction: Codec error for RPC-CORE: submitAndWatchExtrinsic(extrinsic: Extrinsic): ExtrinsicStatus:: 1011: Error: wasm 'unreachable' instruction executed.

@TarikGul
Copy link
Member

This Option being part of the hex is intended, in addition to that we don't want to add any breaking changes to the API, definitely not with the Signer.

Example that its working as intended:

    const signer = new GenericSignerPayload(api.registry, {
        address: '5DTestUPts3kjeXSTMyerHihn1uwMfLj8vU8sqF7qYrFabHE',
        assetId: { parents: 0, interior: { x2: [{ palletInstance: 50 }, { generalIndex: 123 }] } },
        blockHash: '0xde8f69eeb5e065e18c6950ff708d7e551f68dc9bf59a07c52367c0280f805ec7',
        blockNumber: '0x231d30',
        era: api.registry.createType('ExtrinsicEra', { current: 2301232, period: 200 }),
        genesisHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
        metadataHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
        method: api.tx.system.remark('0x00').method,
        mode: 1,
        nonce: 0x1234,
        signedExtensions: ['CheckNonce'],
        tip: 0x5678,
        version: 4,
        withSignedTransaction: true
    });
    
    console.log(signer.assetId.toHex())

This returns:

{
  specVersion: '0x00000000',
  transactionVersion: '0x00000000',
  address: 'DyGYCpGXE4gVJLtE4nhkF5hvcCBAL9ubJJtGVX5KM4kKpaW',
  assetId: '0x010002043205ed01',
  blockHash: '0xde8f69eeb5e065e18c6950ff708d7e551f68dc9bf59a07c52367c0280f805ec7',
  blockNumber: '0x00231d30',
  era: '0x0703',
  genesisHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
  metadataHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
  method: '0x00000400',
  mode: 1,
  nonce: '0x00001234',
  signedExtensions: [ 'CheckNonce' ],
  tip: '0x00000000000000000000000000005678',
  version: 4,
  withSignedTransaction: true
}

I'll comment further in the issue.

@Chralt98
Copy link
Author

Closed, because this PR #6074 (comment) fixed the issue.

@Chralt98 Chralt98 closed this Jan 30, 2025
@Chralt98 Chralt98 deleted the patch-1 branch January 30, 2025 12:47
@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Feb 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decode of assetId hex string fails for signed payload because of Option
3 participants