-
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
Remove TransactionHash and hash and update to use TransactionComputer.computeTransactionHash #566
Remove TransactionHash and hash and update to use TransactionComputer.computeTransactionHash #566
Conversation
….computeTransactionHash
src/abi/smartContract.spec.ts
Outdated
const computer = new TransactionComputer(); | ||
const hashString = computer.computeTransactionHash(deployTransaction); |
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.
Needed? We have a hash
above. Sorry if mistaken.
src/abi/interaction.spec.ts
Outdated
transaction.getHash().toString(), | ||
"3579ad09099feb9755c860ddd225251170806d833342e912fccdfe2ed5c3a364", | ||
); | ||
const computer = new TransactionComputer(); |
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 can hold result of await provider.sendTransaction(transaction);
and don't use this computer? (serves as a more real-world example, I think).
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.
Below, as well.
src/abi/interaction.spec.ts
Outdated
"3579ad09099feb9755c860ddd225251170806d833342e912fccdfe2ed5c3a364", | ||
); | ||
const computer = new TransactionComputer(); | ||
let hashString = computer.computeTransactionHash(transaction); |
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've simply been called hash
.
const serializer = new ProtoSerializer(); | ||
const buffer = serializer.serializeTransaction(transaction); | ||
const hash = createTransactionHasher(TRANSACTION_HASH_LENGTH).update(buffer).digest("hex"); | ||
|
||
return Buffer.from(hash, "hex"); | ||
return Buffer.from(hash, "hex").toString("hex"); |
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 hash
is already a hex string so there's no need to convert it again. Right?
No description provided.