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

Serialized transaction does not match original txJSON due to wrong serialization / deserialization of paths #1347

Closed
tadejgolobic opened this issue Jan 5, 2021 · 12 comments · Fixed by #1382
Assignees

Comments

@tadejgolobic
Copy link
Contributor

We have an issue with signing transaction that has paths field specified. It looks like that paths object needs to have all three fields specified (account, issuer and currency). In case one of them is not defined / not provided, then ripple-lib will throw following error:
Serialized transaction does not match original txJSON. See error.data.

Lets look at this example:

const pathfind = {
     source: {
         address: 'rNf1i8YPXbV3Nh4FDsJiyENqMmGN8PnaLR',
         maxAmount: {
             currency: 'drops',
             amount: '100'
         }
     },
     destination: {
       address: 'rKT4JX4cCof6LcDYRz8o3rGRu7qxzZ2Zwj',
       amount: {
         value: '1',
         currency: 'USD',
         counterparty: 'rVnYNK9yuxBz4uP8zC8LEFokM2nqH3poc'
       }
     }
 }

 xrp.getPaths(pathfind).then(data => {
     pathfind.paths = data[0].paths;
     xrp.preparePayment('rNf1i8YPXbV3Nh4FDsJiyENqMmGN8PnaLR', pathfind).then(ret => {
         const signed = xrp.sign(ret.txJSON, wallet.secret);
     }).catch(err => {
         console.log('ERR: ', err);
     })
 }).catch(err => {
     console.log('ERR: ', err);
 })

With this example we get error as described above. We also found out, that this happens because of ripple-binary-codec.

If we check following code:


const txJSON = {
    Fee: "45",
    Amount: "1000000",
    Account: "rMqzrqJgSVzfKEJNyTxJnkpybMBjTTfuef",
    Destination: "rVnYNK9yuxBz4uP8zC8LEFokM2nqH3poc",
    TransactionType: "Payment",
    Paths: [
        [
            { account: 'rVnYNK9yuxBz4uP8zC8LEFokM2nqH3poc'}
        ]
    ]
}


const encoded = binaryCodec.encode(txJSON)
const decoded = binaryCodec.decode(encoded);

console.log('DECODED: ', decoded['Paths']);

this is the output:

"DECODED: " [
  [
    {
      account: 'rVnYNK9yuxBz4uP8zC8LEFokM2nqH3poc',
      issuer: undefined,
      currency: undefined
    }
  ]
]

So issue is that everytime, paths will be defined and one of the PathFind fields will not be defined, we cannot sign transaction. I would also like to point out, that in paths, we cannot pass undefined values from our side, to ripple-lib, because txJSON must be passed as string, and we cannot stringify undefined values.

I would also like to ask following question:
Is it absolutely necessary to do encode, and then immediately decode here: https://github.com/ripple/ripple-lib/blob/develop/src/transaction/sign.ts#L59 (decode is performed inside checkTxSerialization function)

@alexdupre
Copy link

This looks like a serious regression, that makes often impossible to use the result of the getPaths to prepare and sign a payment. It started happening in the 1.9.0 release (that updated the ripple-binary-codec dependency).

@intelliot
Copy link
Collaborator

If you use ripple-lib 1.8.2, does everything work fine?

@tadejgolobic
Copy link
Contributor Author

@alexdupre @intelliot this was issue in ripple-binary-codec. Here is issue ripple/ripple-binary-codec#117

I already opened PR and PR was also merged. Should be fixed in next release.

@nickvujasin
Copy link

Hey guys, was running into this issue with 1.9 and just found this thread. I'm assuming it isn't fixed in 1.9.1 because I just upgraded and it's still occurring. Do you have a time frame as to when this will be released?

@alexdupre
Copy link

alexdupre commented Mar 10, 2021

If you use ripple-lib 1.8.2, does everything work fine?

Yes. I'm surprised you released another version that includes this severe issue.

@tadejgolobic
Copy link
Contributor Author

tadejgolobic commented Mar 10, 2021

Apparently there is already a pull request #1376

@nickvujasin
Copy link

@intelliot Hi Elliot, I was hoping you could give us some insight as to when this will be released. I see that this issue has been open since January and the PR to fix this has been out there for 14 days. I agree with Alex that this is a critical piece of functionality, it is what the XRPL was designed to do, payments and right now 1.9 and 1.9.1 can't make them. Thanks, Nick

@intelliot
Copy link
Collaborator

intelliot commented Mar 13, 2021

Thank you all for reporting this, especially @tadejgolobic for the code sample.

This should be fixed in version 1.9.2

@nickvujasin
Copy link

@intelliot Hey Elliot, thank you for getting this fix in and with an immediate release. I very much appreciate it. Nick

@Satosh-J
Copy link

Hi, intelliot.
I have ValidationError: Serialized transaction does not match original txJSON. See error.data

const transactionBlob = {
		"TransactionType": "NFTokenAcceptOffer",
		"Account": wallet.classicAddress,
		"SellOffer": tokenOfferIndex,
	}

const tx = await client.submitAndWait(transactionBlob, { wallet })

this is NFTokenOffer accept tx.
This is the error data

{
    "name": "ValidationError",
    "data": {
        "decoded": {
            "TransactionType": "NFTokenAcceptOffer",
            "Flags": 0,
            "Sequence": 1824368,
            "LastLedgerSequence": 2200579,
            "Fee": "12",
            "Account": "rNZ8EFNxCTf2sZTVExQgayqPRsGZK9AgQ8"
        },
        "tx": {
            "TransactionType": "NFTokenAcceptOffer",
            "Account": "rNZ8EFNxCTf2sZTVExQgayqPRsGZK9AgQ8",
            "SellOffer": "A6BC70E852CBED46BD1DF60909AC1D4DD328C588BE3F7CF774229088BCFBD665",
            "Flags": 0,
            "Sequence": 1824368,
            "Fee": "12",
            "LastLedgerSequence": 2200579
        }
    }
}

Please let me know the reason. Thanks.

@JST5000
Copy link
Contributor

JST5000 commented May 17, 2022

@Satosh-J I think the issue is that you have a field "SellOffer", when it should be "NFTokenSellOffer" - https://xrpl.org/nftokenacceptoffer.html#nftokenacceptoffer

(This was a recent change to the NFT naming conventions to standardize it)

@intelliot
Copy link
Collaborator

intelliot commented May 17, 2022

@Satosh-J If the problem persists, please create a new issue here. (This existing issue is an old one which was resolved/closed)

n/m, just noticed #2002 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants