-
Notifications
You must be signed in to change notification settings - Fork 37
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
Check if isCompleted is undefined #315
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import { TokenTransfer } from "./tokenTransfer"; | |
import { Transaction } from "./transaction"; | ||
import { TransactionPayload } from "./transactionPayload"; | ||
import { TransactionWatcher } from "./transactionWatcher"; | ||
import { ProxyNetworkProvider } from "@multiversx/sdk-network-providers-next"; | ||
|
||
describe("test transaction", function () { | ||
let alice: TestWallet, bob: TestWallet; | ||
|
@@ -15,8 +16,8 @@ describe("test transaction", function () { | |
({ alice, bob } = await loadTestWallets()); | ||
}); | ||
|
||
it("should send transactions", async function () { | ||
this.timeout(30000); | ||
it("should send transactions and wait for completion", async function () { | ||
this.timeout(70000); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. Because the completion checks are now much more strict, multiversx/mx-chain-proxy-go#386, and the wait time is increased. Let's add links towards the following PRs in the description: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
|
||
let provider = createLocalnetProvider(); | ||
let watcher = new TransactionWatcher(provider); | ||
|
@@ -62,6 +63,40 @@ describe("test transaction", function () { | |
assert.deepEqual(TokenTransfer.egldFromAmount(85).valueOf(), newBalanceOfBob.minus(initialBalanceOfBob)); | ||
}); | ||
|
||
it("should send transaction and wait for completion using the new proxy provider", async function () { | ||
this.timeout(70000); | ||
|
||
let provider = createLocalnetProvider(); | ||
let newProvider = new ProxyNetworkProvider("http://localhost:7950", { timeout: 5000 }); | ||
let watcher = new TransactionWatcher({ | ||
getTransaction: async (hash: string) => { return await newProvider.getTransaction(hash, true) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This snippet should be added in the PR description (and in the release notes). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in the PR description. Will also add in the release nodes when it's time. |
||
}); | ||
|
||
let network = await provider.getNetworkConfig(); | ||
|
||
await alice.sync(provider); | ||
await bob.sync(provider); | ||
let initialBalanceOfBob = new BigNumber(bob.account.balance.toString()); | ||
|
||
let transactionOne = new Transaction({ | ||
sender: alice.address, | ||
receiver: bob.address, | ||
value: TokenTransfer.egldFromAmount(42), | ||
gasLimit: network.MinGasLimit, | ||
chainID: network.ChainID | ||
}); | ||
|
||
transactionOne.setNonce(alice.account.nonce); | ||
await signTransaction({ transaction: transactionOne, wallet: alice }); | ||
await provider.sendTransaction(transactionOne); | ||
await watcher.awaitCompleted(transactionOne); | ||
|
||
await bob.sync(provider); | ||
let newBalanceOfBob = new BigNumber(bob.account.balance.toString()); | ||
|
||
assert.deepEqual(TokenTransfer.egldFromAmount(42).valueOf(), newBalanceOfBob.minus(initialBalanceOfBob)); | ||
}); | ||
|
||
it("should simulate transactions", async function () { | ||
this.timeout(20000); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { AsyncTimer } from "./asyncTimer"; | ||
import { Err, ErrExpectedTransactionEventsNotFound, ErrExpectedTransactionStatusNotReached } from "./errors"; | ||
import { Err, ErrExpectedTransactionEventsNotFound, ErrExpectedTransactionStatusNotReached, ErrIsCompletedFieldIsMissingOnTransaction } from "./errors"; | ||
import { ITransactionFetcher } from "./interface"; | ||
import { ITransactionEvent, ITransactionOnNetwork, ITransactionStatus } from "./interfaceOfNetwork"; | ||
import { Logger } from "./logger"; | ||
|
@@ -69,7 +69,13 @@ export class TransactionWatcher { | |
* Waits until the transaction is completely processed. | ||
*/ | ||
public async awaitCompleted(transaction: ITransaction): Promise<ITransactionOnNetwork> { | ||
const isCompleted = (transactionOnNetwork: ITransactionOnNetwork) => transactionOnNetwork.isCompleted; | ||
const isCompleted = (transactionOnNetwork: ITransactionOnNetwork) => { | ||
if (transactionOnNetwork.isCompleted === undefined) { | ||
throw new ErrIsCompletedFieldIsMissingOnTransaction(); | ||
} | ||
return transactionOnNetwork.isCompleted | ||
}; | ||
|
||
const doFetch = async () => await this.fetcher.getTransaction(transaction.getHash().hex()); | ||
const errorProvider = () => new ErrExpectedTransactionStatusNotReached(); | ||
|
||
|
@@ -155,6 +161,10 @@ export class TransactionWatcher { | |
} catch (error) { | ||
Logger.debug("TransactionWatcher.awaitConditionally(): cannot (yet) fetch data."); | ||
|
||
if (error instanceof ErrIsCompletedFieldIsMissingOnTransaction) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is correct. @CiprianDraghici can confirm, as well. |
||
throw error; | ||
} | ||
|
||
if (!(error instanceof Err)) { | ||
throw error; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right for now (to reference the beta version), since it's used for tests only (dev dependency). Once
sdk-core
is compatible with both the old and the newsdk-network-providers
(as of this PR), we can make a proper release there, as well. And afterwards, come back here to adjust the reference.For
sdk-core
, let's bump the minor version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, got it. So after this PR is merged we make a proper release for sdk-network-providers, then come back here and reference the proper release. Will also make a new release for sdk-core increasing the minor version. 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, release of this adjustment of sdk-core, then release sdk-network-providers. Afterwards, come back in
sdk-core
to adjust the version (this can be delayed anyway).