Skip to content

App to not run messages on CheckTx #1120

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

Closed
rigelrozanski opened this issue Jun 2, 2018 · 9 comments
Closed

App to not run messages on CheckTx #1120

rigelrozanski opened this issue Jun 2, 2018 · 9 comments
Assignees

Comments

@rigelrozanski
Copy link
Contributor

May be a duplicate issue?

As discussed many times previously we need checkTx to only check that fees can be paid, from there on the messages should not run validate basic to waste unpaid time - the transactions should then pass checktx and be allowed to fail on delivertx.

@rigelrozanski
Copy link
Contributor Author

rigelrozanski commented Jun 2, 2018

As a part of this validate basic should be removed from the message struct - those validations no longer need to be separated out and should just be brought into msg handler

@davekay100
Copy link
Contributor

@rigelrozanski I've started to work on implementing this fix in this WIP PR https://github.com/cosmos/cosmos-sdk/pull/1172/files . I got a few questions outlined in the PR if you get a chance to answer them.

@davekay100
Copy link
Contributor

this issue is related #518

@rigelrozanski
Copy link
Contributor Author

Thanks as per newest discussion within #518 we should implement this within the baseapp level such that handlers have no need to call IsCheckTx - in the future we will implement new handlers which can be referenced during checkTx however there is no need before launch

@rigelrozanski
Copy link
Contributor Author

@davekaj I've commented in detail on your PR questions - also see my PR comments - thanks for putting in the time here!

@rigelrozanski
Copy link
Contributor Author

let's keep validate basic! nice to have a check on the message without requiring access to state - good convention

@ebuchman
Copy link
Member

ebuchman commented Jul 1, 2018

Being addressed in #1172

@ebuchman ebuchman mentioned this issue Jul 6, 2018
7 tasks
@alexanderbez
Copy link
Contributor

@ebuchman can we close this?

@ebuchman
Copy link
Member

No it hasn't actually been addressed yet :)

@cwgoes cwgoes self-assigned this Jul 18, 2018
@cwgoes cwgoes mentioned this issue Jul 18, 2018
8 tasks
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