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 isSigned null check to handle multi-signed transactions #2308

Merged
merged 6 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions packages/xrpl/src/sugar/submit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { decode, encode } from 'ripple-binary-codec'

import type { Client, SubmitRequest, SubmitResponse, Wallet } from '..'
import { ValidationError, XrplError } from '../errors'
import { Signer } from '../models/common'
import { TxResponse } from '../models/methods'
import { Transaction } from '../models/transactions'
import { hashes } from '../utils'
Expand Down Expand Up @@ -222,10 +223,26 @@ async function waitForFinalTransactionOutcome(
// checks if the transaction has been signed
function isSigned(transaction: Transaction | string): boolean {
const tx = typeof transaction === 'string' ? decode(transaction) : transaction
return (
typeof tx !== 'string' &&
(tx.SigningPubKey != null || tx.TxnSignature != null)
)
if (typeof tx === 'string') {
return false
}
if (tx.Signers != null) {
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- we know that tx.Signers is an array of Signers
const signers = tx.Signers as Signer[]
for (const signer of signers) {
khancode marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line max-depth -- necessary for checking if signer is signed
if (
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- necessary check
signer.Signer.SigningPubKey == null ||
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- necessary check
signer.Signer.TxnSignature == null
) {
return false
}
}
return true
}
return tx.SigningPubKey != null && tx.TxnSignature != null
}

// initializes a transaction for a submit request
Expand Down
30 changes: 27 additions & 3 deletions packages/xrpl/test/client/submit.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/* eslint-disable @typescript-eslint/restrict-template-expressions -- error type thrown can be any */
import { assert } from 'chai'
import cloneDeep from 'lodash/cloneDeep'

import { ValidationError } from '../../src'
import { multisign, ValidationError } from '../../src'
import { Transaction } from '../../src/models/transactions'
import Wallet from '../../src/Wallet'
import rippled from '../fixtures/rippled'
Expand Down Expand Up @@ -56,7 +57,6 @@ describe('client.submit', function () {
const response = await testContext.client.submit(tx, { wallet })
assert(response.result.engine_result, 'tesSUCCESS')
} catch (error) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- error type thrown can be any
assert(false, `Did not expect an error to be thrown: ${error}`)
}
})
Expand Down Expand Up @@ -114,7 +114,31 @@ describe('client.submit', function () {
const response = await testContext.client.submit(signedTxEncoded)
assert(response.result.engine_result, 'tesSUCCESS')
} catch (error) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- error type thrown can be any
assert(false, `Did not expect an error to be thrown: ${error}`)
}
})

it('should submit a multisigned transaction', async function () {
const signerWallet1 = Wallet.generate()
const signerWallet2 = Wallet.generate()
const accountSetTx: Transaction = {
TransactionType: 'AccountSet',
Account: 'rhvh5SrgBL5V8oeV9EpDuVszeJSSCEkbPc',
Sequence: 1,
Fee: '12',
LastLedgerSequence: 12312,
}

testContext.mockRippled!.addResponse('submit', rippled.submit.success)

const signed1 = signerWallet1.sign(accountSetTx, true)
const signed2 = signerWallet2.sign(accountSetTx, true)
const multisignedTxEncoded = multisign([signed1.tx_blob, signed2.tx_blob])

try {
const response = await testContext.client.submit(multisignedTxEncoded)
assert(response.result.engine_result, 'tesSUCCESS')
} catch (error) {
assert(false, `Did not expect an error to be thrown: ${error}`)
}
})
Expand Down