-
Notifications
You must be signed in to change notification settings - Fork 35
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 txid in fromHex #313
Conversation
✅ Deploy Preview for cheery-moxie-4f1121 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
nice improvement, just one nit
scripts/transaction.js
Outdated
@@ -94,6 +94,7 @@ export class Transaction { | |||
shieldSpend = [], | |||
shieldOutput = [], | |||
bindingSig = '', | |||
txid, |
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.
unused parameter in the constructor
scripts/transaction.js
Outdated
@@ -105,11 +106,14 @@ export class Transaction { | |||
this.shieldOutput = shieldOutput; | |||
this.bindingSig = bindingSig; | |||
this.valueBalance = valueBalance; | |||
this.#txid = txid; |
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.
and here too
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.
tACK da03c23
I've been comparing with multiple tests against master
and getting consistent improvements, my wallet is far smaller than yours so the magnitude is much less (about a 10% speed increase) but definitely noticeable in the performance captures.
Abstract
MPW was spending a lot of time serializing the transaction when calculating the txid.
To prevent this, we can calculate the txid during the serialization, when we have the hex.
This is the benchmark before the PR, on a very big testnet wallet:
This is after:
Testing
Test that the general functionality works