GetSpeckle / speckle-browser-extension

Universal browser extension for Web 3 and the Polkadot ecosystem
GNU Affero General Public License v3.0
27 stars 6 forks source link

Unknown error code 1010 #140

Closed hskang9 closed 3 years ago

hskang9 commented 3 years ago

image

After the recent update for Polkadot JS API, the error shows whenever the transaction happens. @jacogr can you describe error with the code 1010?

jacogr commented 3 years ago

It is anything signature related -

i.e. anything that makes up the transaction part. What is a recent API update? the only format changes on extrinsics in the last 6 months has been the need for the transaction version (which is obviously catered for by the new signed extension that the metadata specifies).

hskang9 commented 3 years ago

Ok then it seems the problem comes with the transaction payload. I think the problem here is now either speckle-browser-extension currently gets metadata from its state or value for transaction payload is wrong. @jacogr can you check if there is something wrong with the current signer payload value here?

jacogr commented 3 years ago

Are you injecting the correct extensions? e.g. https://github.com/polkadot-js/extension/blob/master/packages/extension-base/src/background/RequestExtrinsicSign.ts#L19

I honestly don't follow the linked code at all, it seems you are duplicating what is in the API for construction and then passing to sign, which I'm not sure about at all, it is very possible that the objects there are not done correctly.

fyang1024 commented 3 years ago

Hi @jacogr Yes, the signer does inject extensions. https://github.com/GetSpeckle/speckle-browser-extension/blob/8953b00f8553557ae69f436f74e2aad3dcb43a71/src/ts/background/services/keyring-vault.ts#L295 And the extensions is passed in payload https://github.com/GetSpeckle/speckle-browser-extension/blob/8953b00f8553557ae69f436f74e2aad3dcb43a71/src/ts/components/transaction/Send.tsx#L180 And the payload got it from here https://github.com/GetSpeckle/speckle-browser-extension/blob/8953b00f8553557ae69f436f74e2aad3dcb43a71/src/ts/constants/chains.ts#L76

But it seems unnecessary for payload to get it from chain registry and pass it to the signer, as the signer can get it from chain registry directly

jacogr commented 3 years ago

Ok, so the above code, I cannot follow at all - like I mentioned, it seems like a duplication of something the the API and I cannot tracks and follow it at all.

So what I would suggest -

jacogr commented 3 years ago

Any update in terms of hex?

fyang1024 commented 3 years ago

Hi @jacogr @hskang9 and I will work together on this issue this Saturday. Hopefully we can solve it, otherwise we'll log the hex payload and signature and seek your advice. Thank you

jacogr commented 3 years ago

It will be quite quick to decode it (20+ mins), so please do share. It may show some light.

fyang1024 commented 3 years ago

Hi @jacogr, here are the log

hex payload: 0xa00400c447b9ce86e7e271971500b09e648457daf4c8126df1783cfcf71d93ce6f5b1e0700e40b540200ec00ea070000b0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafeb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe signature: 0x01a209d4c849d516df742b70e4a7791634a4d7d3fdc530a7ac3ee9d55567c3a76a539c0c433b108459b7b6180d93e903714038bfb61fef5926162b21aeb9f2d788

Which tool should we use to decode them? Thank you

jacogr commented 3 years ago

Manual decoding, no tool. Will check in around 60 mins.

jacogr commented 3 years ago

Just looking, not sure the first byte should be there for the part that is signed - the length encoding for the payload part of the call should not be there.

hskang9 commented 3 years ago

👀ok

jacogr commented 3 years ago

(Byte 2 and 3 is correct, since it refers to a transfer). So if that is what is signed the encoding format is incorrect - the toU8a should be done without length encoding.

jacogr commented 3 years ago

I may be wrong will check shortly against the extensions for the chain which determines this all.

jacogr commented 3 years ago

Indeed. Your encoding is incorrect.

https://github.com/polkadot-js/api/blob/master/packages/types/src/extrinsic/ExtrinsicPayload.spec.ts#L43

fyang1024 commented 3 years ago

Thank you @jacogr we'll try to fix it today.

fyang1024 commented 3 years ago

Hi @jacogr We did the same transaction (transfer 10 milliKSM from Account A to B) using \Polkadot JS app and Speckle and logged the SignerPayloadJSON, then we compared them and found some differences

The SignerPayloadJSON generated from polkadot js app looks like this

{
  address: "CadXL6ZfQ8JrrV2YF65U1jrRxZoT5qAyaqm3JY3kBAujNqq"
  blockHash: "0xd91339cc06ee0504b187886969e415758c58a0724f64b60fdcbbb092f7745fa7"
  blockNumber: "0x004c6fc7"
  era: "0x7500"
  genesisHash: "0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe"
  method: "0x0403c447b9ce86e7e271971500b09e648457daf4c8126df1783cfcf71d93ce6f5b1e0700e40b5402"
  nonce: "0x00000041"
  signedExtensions: Array(7)
    0: "CheckSpecVersion"
    1: "CheckTxVersion"
    2: "CheckGenesis"
    3: "CheckMortality"
    4: "CheckNonce"
    5: "CheckWeight"
    6: "ChargeTransactionPayment"
  specVersion: "0x000007ea"
  tip: "0x00000000000000000000000000000000"
  transactionVersion: "0x00000003"
  version: 4
}

While the SignerPayloadJSON generated by Speckle looks like this:

{
  address: "CadXL6ZfQ8JrrV2YF65U1jrRxZoT5qAyaqm3JY3kBAujNqq"
  blockHash: "0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe"
  blockNumber: "0"
  era: "0x00"
  genesisHash: "0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe"
  method: "0x0400c447b9ce86e7e271971500b09e648457daf4c8126df1783cfcf71d93ce6f5b1e0700e40b5402"
  nonce: "0x00000041"
  signedExtensions: Array(7)
    0: "CheckVersion"
    1: "CheckGenesis"
    2: "CheckEra"
    3: "CheckNonce"
    4: "CheckWeight"
    5: "ChargeTransactionPayment"
    6: "CheckBlockGasLimit"
  specVersion: "0x000007ea"
  tip: "0"
  transactionVersion: "0x00000003"
  version: 4
}

We notice that there are 3 differences:

  1. blockNumber: "0x004c6fc7" vs "0"
  2. era: "0x7500" vs "0x00"
  3. method: "0x0403c447b9ce86e7e271971500b09e648457daf4c8126df1783cfcf71d93ce6f5b1e0700e40b5402" vs "0x0400c447b9ce86e7e271971500b09e648457daf4c8126df1783cfcf71d93ce6f5b1e0700e40b5402"

We get blockNumber with api.query.system.number(), which gives "0" We get era and method from extrinsic, while we get extrinsic with api.tx.balances.transfer(this.state.toAddress, amountBn)

I think the error comes from the 3 differences. We don't know why api.query.system.number() doesn't give a correct blockNumber, and why extrinsic doesn't give correct era or method.

jacogr commented 3 years ago

The signed extensions are different.

Yes, you cannot retrieve the blockNumber via system.number that is used while building the block in the node (it is a scratchpad storage entry) - see the Rust code, it is expected to not yield values.

hskang9 commented 3 years ago

@jacogr Is there a way to get ExtrinsicEra from polkadot-js api? Or would it not matter to send transaction?

jacogr commented 3 years ago

The issue here is that the code seems to duplicate/redo what is in the extrinsic definitions by managing own payloads. If you just get the API itself to take care of it for you, you don't have these issues. You certainly don't need to worry about transaction versions, eras, nonces, etc. via any .signAndSend operation. The only hook should b to sign a payload.

I'm not sure I understand the question - the eras are absolutely critical, alongside the blockHash for the era constructed.

hskang9 commented 3 years ago

True. The team rather builds extrinsic from botttom up and maybe we need to change how we build transaction. We need to use API.

Ok era is critical alongside blockHash.

hskang9 commented 3 years ago

I expect the code will be a lot clear when it uses api. I am doing it.