Skip to content
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

Make simulation independent of private key #2151

Closed
ValarDragon opened this issue Aug 25, 2018 · 9 comments
Closed

Make simulation independent of private key #2151

ValarDragon opened this issue Aug 25, 2018 · 9 comments

Comments

@ValarDragon
Copy link
Contributor

Summary

We should make the simulation for gas calculation independent of the private key. ref #2047 (comment)

Reasoning is: increases applicability of side channel attacks, increases private key exposure, weird UI (approving 2 txs on ledger), and if that signature is recovered, it can be propagated to more of the network which will burn your fees. (as you get an out of gas error)

Once this is done, we can enable simulation by default in the CLI.

@alexanderbez
Copy link
Contributor

Think this should be #prelaunch

@ValarDragon
Copy link
Contributor Author

ref #2047 (comment)

I don't understand the push for this to be prelaunch, let alone next release. (Note this contradicts what I said earlier in the issue) Its a nice to have (and a really cool thing at that), but something we can roll out in a non-breaking manner. The more features we keep trying to add prelaunch that aren't necessary, the longer it will take to launch.

@ValarDragon
Copy link
Contributor Author

Per discussion in sdk meeting, this being prelaunch is tied to our decision on gas, which is dependent on iavl benchmarks.

@jackzampolin
Copy link
Member

Is this still prelaunch? We don't have any IAVL benchmarks rn...

@ValarDragon
Copy link
Contributor Author

I don't follow with any of the new labels? Was simulation of txs for gas made default without doing this? If so I think we should undo that decision and wait until this is complete.

@alessio
Copy link
Contributor

alessio commented Jan 23, 2019

Are we 100% sure this is launch blocker?

@jackzampolin
Copy link
Member

Is this still an issue? Didn't you just fix this in the LCD PR you just pushed @alexanderbez

@alexanderbez
Copy link
Contributor

Yes it is @jackzampolin -- although I'm not quite sure what @ValarDragon means.

@jackzampolin
Copy link
Member

Going to go ahead and close this issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants