ripple / ripple-binary-codec

Convert between json and hex representations of transactions and ledger entries on the XRP Ledger. Moved to: https://github.com/XRPLF/xrpl.js/tree/develop/packages/ripple-binary-codec
https://github.com/XRPLF/xrpl.js/tree/develop/packages/ripple-binary-codec
ISC License
19 stars 45 forks source link

Cannot Sign Ledger Account Set Transactions #151

Closed dangell7 closed 2 years ago

dangell7 commented 2 years ago

If I try to sign the following tx json it fails with Ledger device: Missing critical parameter (0x6800)

{
    Account: "OMMITED"
    Domain: "OMMITED"
    Fee: "12"
    Flags: 65536
    LastLedgerSequence: 67379294
    Sequence: 67377767
    SetFlag: 8
    SigningPubKey: "OMMITED"
    TickSize: 6
    TransactionType: "AccountSet"
    TransferRate: 1030000000
}

https://github.com/LedgerHQ/app-xrp/issues/18

If I remove the Flags parameter it succeeds in the signature but fails to decode correctly.

When I try to use decode from the ripple-codec, on a signed transaction response, I receive the following error.

Cannot read properties of undefined (reading 'name')

https://github.com/LedgerHQ/app-xrp/issues/17

Thank you!

natenichols commented 2 years ago

Hi @dangell7, thanks for opening an issue.

Off the bat, I don't see anything suspicious in the ripple-binary-codec behavior.

{
    Account: "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
    Domain: "1234555445",
    Fee: "12",
    Flags: 65536,
    LastLedgerSequence: 67379294,
    Sequence: 67377767,
    SetFlag: 8,
    SigningPubKey: "1234567890",
    TickSize: 6,
    TransactionType: "AccountSet",
    TransferRate: 1030000000,
}

I am able to encode and decode this transaction without issue, and the original and decoded transaction are equivalent.

You can use this tool to help you visualize decoding transactions on xrp ledger and see exactly where the encoded bit is failing.

I might dig into the ledgerHQ code a bit later, but could you provide a little more context about how you're using the binary-codec? This very well still could be an issue, so I'm interested to gather more information.

Thanks again! Nathan

dangell7 commented 2 years ago

Nathan, thank you for your FAST response.

The tx above is good, its the signature that ledger returns that is the issue. (I copied from the original post on ledger so that was my confusion error)

Here is the full ledger code:

export const signXRPTransaction = async (tx) => {
  const cloneTx = tx;
  const transport = await TransportWebUSB.open();
  const xrp = new Xrp(transport);
  const { publicKey } = await xrp.getAddress("44'/144'/1'/0/0", false);
  cloneTx.SigningPubKey = publicKey.toUpperCase();
  const txBlob = serializeTx(cloneTx).toUpperCase();
  const signedTx = await xrp.signTransaction(
    "44'/144'/1'/0/0",
    txBlob
  );
  cloneTx.signedTransaction = {};
  cloneTx.signedTransaction.hash = txBlob;
  cloneTx.signedTransaction.tx_blob = signedTx.toUpperCase();
  return cloneTx;
};

The error I get is with the signedTransaction.tx_blob on the submit function using the xrpl-js, which inside uses the ripple-codec.

They have me doing something first, so let me check if that works. Then I will report back.

dangell7 commented 2 years ago

After updating the tfFullyCanonicalSig flag I'm able to sign the transaction however I do still get the Cannot read properties of undefined (reading 'name') error.

Here is the tx json:

{
    Account: "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
    Domain: "1234555445",
    Fee: "12",
    Flags: 2147483648,
    LastLedgerSequence: 67379294,
    Sequence: 67377767,
    SetFlag: 8,
    SigningPubKey: "1234567890",
    TickSize: 6,
    TransactionType: "AccountSet",
    TransferRate: 1030000000,
}

I will use the tool you gave me to decipher the decode function results. Also here is the full error stack:

TypeError: Cannot read properties of undefined (reading 'name')
    at Function.__webpack_modules__../node_modules/ripple-binary-codec/dist/types/st-object.js.STObject.fromParser (st-object.js:81)
    at BinaryParser.__webpack_modules__../node_modules/ripple-binary-codec/dist/serdes/binary-parser.js.BinaryParser.readType (binary-parser.js:142)
    at readJSON (binary.js:31)
    at binaryToJSON (binary.js:40)
    at decode (index.js:14)
    at isSigned (submit.js:81)
    at submit.js:87
    at Generator.next (<anonymous>)
    at submit.js:8
    at new Promise (<anonymous>)
natenichols commented 2 years ago

Were you able to sign and submit w/ the tfFullyCanonicalSig flag set?

dangell7 commented 2 years ago

Only sign.

natenichols commented 2 years ago

Whats the error message you get when you try to submit to rippled?

dangell7 commented 2 years ago
TypeError: Cannot read properties of undefined (reading 'name')
    at Function.__webpack_modules__../node_modules/ripple-binary-codec/dist/types/st-object.js.STObject.fromParser (st-object.js:81)
    at BinaryParser.__webpack_modules__../node_modules/ripple-binary-codec/dist/serdes/binary-parser.js.BinaryParser.readType (binary-parser.js:142)
    at readJSON (binary.js:31)
    at binaryToJSON (binary.js:40)
    at decode (index.js:14)
    at isSigned (submit.js:81)
    at submit.js:87
    at Generator.next (<anonymous>)
    at submit.js:8
    at new Promise (<anonymous>)
dangell7 commented 2 years ago
{
  Account: 'OMMITTED',
  Domain: '6578616D706C652E636F6D',
  Fee: '12',
  Flags: 2147483648,
  LastLedgerSequence: 67415036,
  Sequence: 67377771,
  SetFlag: 8,
  SigningPubKey: 'OMMITTED',
  TickSize: 6,
  TransactionType: 'AccountSet',
  TransferRate: 1030000000,
  signedTransaction: {
    hash: 'OMMITTED',
    tx_blob: '30450221008854F091FFD95603025DCD789FB9C5BE2951CB8C2A17048F9596D7B2EE746D8F02200F4703A140EF56F842574CFE341EB90AEE5A5DC42F6CAD58EAF5D7DD89F3C7EA',
  }
}

The tx blob is what I believe is failing.. I'm not sure it matters that I pasted it, as it won't submit anyways....

natenichols commented 2 years ago

Ahh, the xrp.signTransaction returns the signature, not the signed transaction. Try something like:

export const signXRPTransaction = async (tx) => {
  const cloneTx = tx;
  const transport = await TransportWebUSB.open();
  const xrp = new Xrp(transport);
  const { publicKey } = await xrp.getAddress("44'/144'/1'/0/0", false);
  cloneTx.SigningPubKey = publicKey.toUpperCase();
  const txBlob = serializeTx(cloneTx).toUpperCase();
  const signature = await xrp.signTransaction(
    "44'/144'/1'/0/0",
    txBlob
  );
  cloneTx.Signature = signature
  return encode(cloneTx);
};
natenichols commented 2 years ago

Please note that this code has not been tested/hasn't been run. IDK if it works as intended, please test it out for yourself and your use case.

But it gets the point across that the ledger device returns the signature, not the encoded and signed transaction. Please let me know if this works

Best, Nathan

dangell7 commented 2 years ago

This makes perfect sense. lol

I'll try it out but I'm sure that was my mistake.