-
Notifications
You must be signed in to change notification settings - Fork 45
Remove transation outcome and fix changes #539
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
Conversation
| export class SmartContractResult { | ||
| sender: string; | ||
| receiver: string; | ||
| raw: Record<string, 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.
I think this class should have been moved as well, It's fine if it's planned for a future PR.
| }); | ||
|
|
||
| const txOutcome = new TransactionOutcome({ smartContractResults: [scResult], logs: logs }); | ||
| const txOutcome = new TransactionOnNetwork({ smartContractResults: [scResult], logs: logs }); |
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.
Could be renamed.
| } | ||
| const address = Buffer.from(event.topics[0]); | ||
| return Address.fromBuffer(address).bech32(); | ||
| return new Address(address).bech32(); |
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.
toBech32 could have been used.
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 could have been named transactionOnNetwork.ts.
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.
are these classes still used somewhere?
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 will remove them in the next pr when I remove the result parsers
| } | ||
| const address = Buffer.from(event.topics[3]); | ||
| return Address.fromBuffer(address).bech32(); | ||
| return new Address(address).bech32(); |
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.
use toBech32 here, as well.
| } | ||
|
|
||
| async awaitCompletedIssueFungible(txHash: string): Promise<IESDTIssueOutcome[]> { | ||
| async awaitCompletedIssueFungible(txHash: string): Promise<{ tokenIdentifier: 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.
So for output objects we don't define types / resources, only for inputs? 💭
| @@ -1,688 +0,0 @@ | |||
| import { assert } from "chai"; | |||
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.
We should add a future task to re-write these system tests for the new factories / controllers (nice to have).
| * Legacy method, use the "sender" property instead. | ||
| */ | ||
| getSender(): IAddress { | ||
| getSender(): 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.
We should add a task to add the deprecation marker for these legacy functions.
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.
done
| private readonly firstTopicIsIdentifier: boolean; | ||
|
|
||
| constructor(options: { abi: IAbi; firstTopicIsIdentifier?: boolean }) { | ||
| constructor(options: { abi: AbiRegistry; firstTopicIsIdentifier?: boolean }) { |
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.
We should add a task to create an alias in v14, so that we have the same name as in PY: Abi. Or to completely rename it.
| data: Uint8Array = new Uint8Array(); | ||
| additionalData: Uint8Array[] = []; |
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.
👍
| Object.assign(this, init); | ||
| } | ||
|
|
||
| static fromHttpResponse(responsePart: { |
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.
In sdk-py, this conversion is defined somewhere else (network providers module).
src/transactionEvents.ts
Outdated
| let result = new TransactionEvent(); | ||
| result.address = new Address(responsePart.address); | ||
| result.identifier = responsePart.identifier || ""; | ||
| result.topics = (responsePart.topics || []).map((topic) => Buffer.from(topic)); |
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.
| findFirstOrNoneTopic(predicate: (topic: Uint8Array) => boolean): Uint8Array | undefined { | ||
| return this.topics.filter((topic) => predicate(topic))[0]; | ||
| } | ||
|
|
||
| getLastTopic(): Uint8Array { | ||
| return this.topics[this.topics.length - 1]; | ||
| } |
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.
If they aren't used anymore, can be dropped (sorry if I'm mistaken, I didn't double check).
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 will remove them in the pr where I remove result parser
No description provided.