TransactionBuilder.fromXDR cannot parse transaction processed earlier by StellarCore with "Assets are not in lexicographic order" error

[orbitlens]

When I try to parse one of the transactions recorded on-chain using the standard TransactionBuilder.fromXDR method, it throws "Assets are not in lexicographic order". The transaction is successfully applied during the execution. This means that JsStellarBase lib and Stellar Core handle transactions validation differently, which may result in errors on the client side, similar to the problem described here.

Environment

• stellar-base package v8.2.2
• Testnet Horizon

To reproduce:

1. Open this transaction link (I also saved it here to preserve after the testnet reset)
2. Copy envelope XDR
3. Call TransactionBuilder.fromXDR or try to import in Laboratory
4. "Assets are not in lexicographic order" error appears

Expected behavior
To my mind the tx validation behavior should be consistent across Core and client libraries to prevent such transaction from busting client interface. Most wallets and other clients by default expect that all information returned from Horizon is not malformed and rarely construct code in such a way to handle possible problems.

Proposed solution

export class LiquidityPoolAsset {
-  constructor(assetA, assetB, fee) {
+  constructor(assetA, assetB, fee, skipValidation = false) {
+    if (!skipValidation) { 
    if (!assetA || !(assetA instanceof Asset)) {
      throw new Error('assetA is invalid');
    }
    if (!assetB || !(assetB instanceof Asset)) {
      throw new Error('assetB is invalid');
    }
    if (Asset.compare(assetA, assetB) !== -1) {
      throw new Error('Assets are not in lexicographic order');
    }
    if (!fee || fee !== LiquidityPoolFeeV18) {
      throw new Error('fee is invalid');
    }
+    }

    this.assetA = assetA;
    this.assetB = assetB;
    this.fee = fee;
  }

  /**
   * Returns a liquidity pool asset object from its XDR ChangeTrustAsset object
   * representation.
   * @param {xdr.ChangeTrustAsset} ctAssetXdr - The asset XDR object.
   * @returns {LiquidityPoolAsset}
   */
  static fromOperation(ctAssetXdr) {
    const assetType = ctAssetXdr.switch();
    if (assetType === xdr.AssetType.assetTypePoolShare()) {
      const liquidityPoolParameters = ctAssetXdr
        .liquidityPool()
        .constantProduct();
      return new this(
        Asset.fromOperation(liquidityPoolParameters.assetA()),
        Asset.fromOperation(liquidityPoolParameters.assetB()),
-        liquidityPoolParameters.fee()
+        liquidityPoolParameters.fee(),
+        true
      );
    }

    throw new Error(`Invalid asset type: ${assetType.name}`);
  }

skipValidation parameter can be omitted from documentation and TypeScript bindings to avoid developers confusion.

[Shaptic]

I don't want to conflate the bug (asset order parsing) which was fixed by #606 with the feature request (skipping validation entirely). I'm going to close this as "fixed," but if you want to open a general discussion about skipping transaction validation on the SDK side, please feel free to open a new issue!