-
Notifications
You must be signed in to change notification settings - Fork 45
Synchronize network providers #557
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
Synchronize network providers #557
Conversation
| }); | ||
|
|
||
| it("counter: should deploy, then simulate transactions using SmartContractTransactionsFactory", async function () { | ||
| it.only("counter: should deploy, then simulate transactions using SmartContractTransactionsFactory", async function () { |
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.
Drop only.
| import { TransactionStatus } from "../transactionStatus"; | ||
| import { ApiNetworkProvider } from "./apiNetworkProvider"; | ||
|
|
||
| describe.only("ApiNetworkProvider Tests", function () { |
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.
Should drop only.
src/transaction.local.net.spec.ts
Outdated
|
|
||
| it("should simulate transactions", async function () { | ||
| this.timeout(20000); | ||
| const JSONbig = require("json-bigint")({ constructorAction: "ignore" }); |
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.
Maybe have the import and a wrapper over jsonBig.parse() within test utils?
src/transactionOnNetwork.ts
Outdated
| } | ||
|
|
||
| static fromSimulateResponse(originalTx: Transaction, response: any): TransactionOnNetwork { | ||
| console.log(response); |
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.
Logging artifact.
| throw new ErrMock("Transaction not found"); | ||
| } | ||
|
|
||
| async getTransactionStatus(txHash: string): Promise<TransactionStatus> { |
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.
Completely removed? Maybe we should still keep it & add it in the specs?
https://github.com/multiversx/mx-sdk-specs/blob/main/network-providers/interface.md
| import { TransactionStatus } from "../transactionStatus"; | ||
| import { ProxyNetworkProvider } from "./proxyNetworkProvider"; | ||
|
|
||
| describe.only("ProxyNetworkProvider Tests", function () { |
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.
only should be removed.
| } | ||
|
|
||
| static fromHttpResponse(payload: any): AccountStorage { | ||
| let result = new AccountStorage(); |
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.
Can also be const.
src/networkProviders/interface.ts
Outdated
| * Fetches a block by nonce or by hash. | ||
| */ | ||
| getNetworkStakeStatistics(): Promise<NetworkStake>; | ||
| getBlock(blockArgs: GetBlockArguments): Promise<BlockOnNetwork>; |
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.
I think this is removed from the interface in sdk-py, and implemented differently in API provider and proxy provider. How should we proceed?
src/networkProviders/resources.ts
Outdated
| } | ||
| } | ||
|
|
||
| export class GetBlockArguments { |
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.
I think this is removed from sdk-py. Does it make sense to drop it here, as well, and also in specs?
| * The gas required by the Network to process a byte of the transaction data. | ||
| */ | ||
| public GasPerDataByte: number; | ||
| public GasPerDataByte: bigint; |
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.
These use a different capitalization. Maybe rename them for consistency (even if that leads to a small breaking change)?
| const response = await this.doPostGeneric(url, transaction); | ||
| const data = response["data"] ?? {}; | ||
| return TransactionOnNetwork.fromSimulateResponse(transaction, data["result"] ?? {}); | ||
| } |
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.
add empty line
| }); | ||
| return await awaiter.awaitOnCondition(transactionHash, condition); | ||
| } | ||
| async awaitTransactionCompleted(transactionHash: string, options?: AwaitingOptions): Promise<TransactionOnNetwork> { |
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.
add empty line
| async getGuardianData(address: Address): Promise<GuardianData> { | ||
| return await this.backingProxyNetworkProvider.getGuardianData(address); | ||
| } |
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.
this is not in the specs, right?
| async getMexPairs(pagination?: IPagination): Promise<PairOnNetwork[]> { | ||
| let url = `mex/pairs`; | ||
| if (pagination) { | ||
| url = `${url}?from=${pagination.from}&size=${pagination.size}`; | ||
| } | ||
|
|
||
| const response: any[] = await this.doGetGeneric(url); | ||
|
|
||
| return response.map((item) => PairOnNetwork.fromApiHttpResponse(item)); | ||
| } |
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.
this is very old and unused I'd say :)
| const deployTxHash = await provider.sendTransaction(deployTransaction); | ||
| const callTxHash = await provider.sendTransaction(smartContractCallTransaction); | ||
|
|
||
| console.log({ deployTxHash, callTxHash }); |
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.
this can be removed
src/networkProviders/resources.ts
Outdated
| export class BlockCoordinates { | ||
| nonce: bigint = 0n; | ||
| hash: string = ""; | ||
| rootHash?: string; |
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.
why is this optional?
| address: Address = Address.empty(); | ||
| key: string = ""; | ||
| value: string = ""; | ||
| constructor(init?: Partial<AccountStorageEntry>) { |
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.
add empty line
src/networkProviders/resources.ts
Outdated
| patienceInMilliseconds: number = DEFAULT_ACCOUNT_AWAITING_PATIENCE_IN_MILLISECONDS; | ||
| } | ||
|
|
||
| export class TransactionCostEstimationResponse { |
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.
this is called TransactionCostResponse in the specs.
| token: Token = new Token({ identifier: "" }); | ||
| amount: bigint = 0n; | ||
| block_coordinates?: BlockCoordinates; | ||
| constructor(init?: Partial<TokenAmountOnNetwork>) { |
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.
add empty line.
| export function importJsonBig(value: string): string { | ||
| return Buffer.from(value, "base64").toString("hex"); | ||
| } | ||
| export const stringifyBigIntJSON = (jsonString: any): any => { |
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.
add empty line
No description provided.