-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fix FIO amounts in parseAction #833
Conversation
src/fio/FioEngine.ts
Outdated
networkFee = dataMaxFee | ||
nativeAmount = `-${networkFee}` // Self-transfer should not affect balance |
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.
Remove comment
} else { | ||
networkFee = isRecipient ? `-${fioAmount}` : fioAmount | ||
const isRecipient = data.to === actor | ||
networkFee = isRecipient ? `0` : fioAmount |
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.
Optional change: since networkFee
is typically dataMaxFee
and these are the exceptions, do the assignment at the beginning with a let
so there's less to mentally process per case
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.
Funny, I purposely kept the initial assignment of networkFee to zero and reassigned it close to nativeAmount to make it easier for myself to understand.
6049ae7
to
1feb2a9
Compare
A couple things are going on here: Consistently set the networkFee and then include that (instead of Fio variables) in nativeAmount calculation. This revealed some amount issues across a few actions. Confirmed with multiple accounts and CSV exports Remove an existingTx check in trnsfiopubky because those shouldn't have duplicate txids since they are just regular sends. I suspect the only existingTx block used is the unstakefio case but will leave the others
1feb2a9
to
863d621
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
none