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

ABCI: add Priority to ResponseCheckTx #1861

Closed
zramsay opened this issue Jul 2, 2018 · 7 comments
Closed

ABCI: add Priority to ResponseCheckTx #1861

zramsay opened this issue Jul 2, 2018 · 7 comments
Labels
C:abci Component: Application Blockchain Interface T:enhancement Type: Enhancement
Milestone

Comments

@zramsay
Copy link
Contributor

zramsay commented Jul 2, 2018

original issue: tendermint/abci#277

TL:DR;

We don't use the fee field and its likely just confusing.

We can add backwards compatible priority (instead of fee) later.

Note priority is better than fee because it lets the app do the math on how to rank order transactions, rather than forcing that into tendermint (ie. if we return fee, priority would be fee/gas)

@zramsay zramsay added the C:abci Component: Application Blockchain Interface label Jul 2, 2018
@zramsay zramsay changed the title Remove Fee (add priority later maybe) ABCI: Remove Fee (add priority later maybe) Jul 2, 2018
@xla xla added the T:enhancement Type: Enhancement label Jul 8, 2018
@xla xla added this to the launch milestone Jul 8, 2018
@ebuchman
Copy link
Contributor

See #1997

@ebuchman ebuchman added this to Planned in current iteration via automation Jul 23, 2018
@melekes melekes self-assigned this Jul 24, 2018
@melekes melekes moved this from Planned to Ongoing in current iteration Jul 24, 2018
melekes added a commit that referenced this issue Jul 24, 2018
Refs #1861

We don't use the fee field and its likely just confusing.

We can add backwards compatible priority (instead of fee) later.

Note priority is better than fee because it lets the app do the math on how to rank order transactions, rather than forcing that into tendermint (ie. if we return fee, priority would be fee/gas)
@melekes melekes mentioned this issue Jul 24, 2018
3 tasks
@melekes melekes moved this from Ongoing to Review Needed in current iteration Jul 24, 2018
xla pushed a commit that referenced this issue Jul 24, 2018
Refs #1861

We don't use the fee field and its likely just confusing.

We can add backwards compatible priority (instead of fee) later.

Note priority is better than fee because it lets the app do the math on how to rank order transactions, rather than forcing that into tendermint (ie. if we return fee, priority would be fee/gas)
@melekes melekes moved this from Review Needed to Done in current iteration Jul 25, 2018
@melekes
Copy link
Contributor

melekes commented Jul 25, 2018

PR with a fix/feature was merged to develop. Will be shipped with 0.22.6 or next breaking release (check the changelog).

@ebuchman
Copy link
Contributor

ebuchman commented Aug 30, 2018

Re-opening because we need to add priority to ResponseCheckTx

@ebuchman ebuchman changed the title ABCI: Remove Fee (add priority later maybe) ABCI: add Priority to ResponseCheckTx Aug 30, 2018
@ebuchman ebuchman reopened this Aug 30, 2018
@melekes melekes removed their assignment Aug 31, 2018
@cwgoes
Copy link
Contributor

cwgoes commented Sep 3, 2018

Is this slated for the next release? We'd like to implement the non-consensus minimum fee downstream for the next SDK release as it's a requirement for GoS.

@ebuchman
Copy link
Contributor

ebuchman commented Sep 4, 2018

@cwgoes afaik the min fee doesnt require co-ordination from tendermint: cosmos/cosmos-sdk#1921 (comment)

@cwgoes
Copy link
Contributor

cwgoes commented Sep 4, 2018

@cwgoes afaik the min fee doesnt require co-ordination from tendermint: cosmos/cosmos-sdk#1921 (comment)

OK, makes sense, thanks!

@tac0turtle
Copy link
Contributor

Closing in favor of issue tendermint/spec#162. Spec work is required prior to implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:abci Component: Application Blockchain Interface T:enhancement Type: Enhancement
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants