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 min_fee be able to be paid/set in multiple denominations #3105

Closed
jackzampolin opened this issue Dec 13, 2018 · 10 comments
Closed

Make min_fee be able to be paid/set in multiple denominations #3105

jackzampolin opened this issue Dec 13, 2018 · 10 comments

Comments

@jackzampolin
Copy link
Member

As a user, I would like to be able to pay/set fees in any denominations of coins native to the chain. To implement this, we would need the following data for each denom:

  • Amount
  • Denomination
  • Gas cost per unit

When entering it in the command line it might look like:

--min_fee=1stake2,5photino4

cc @cwgoes @zmanian @alexanderbez

@alexanderbez
Copy link
Contributor

alexanderbez commented Dec 13, 2018

Just to clarify, the suffix post denom is the gas cost per unit value. It's not the prettiest or most clear UX imho.

--minimum_fees=1steak,5photinos --gas_unit_cost=2steak,4photinos? Requires more sane validation but more clear I think. Maybe there is a better solution.

/cc @ValarDragon

@zmanian
Copy link
Member

zmanian commented Dec 13, 2018

sounds good

@faboweb
Copy link
Contributor

faboweb commented Dec 13, 2018

Just a thought: do we need to do this right now or would it be enough to just go with token for now?

@alexanderbez
Copy link
Contributor

alexanderbez commented Dec 13, 2018

@faboweb it's not required for GoS. I think we should tackle #3118 first (already open PR) and then this. As this will build off of #3118 and requires more changes.

@ValarDragon
Copy link
Contributor

Theres a conflation here.

minimum_fee only exists in the minds of full nodes, as a method to determine whether incoming txs are worth enough to include. See #3044 for how I think that ought to be done. (Also I think that should reside in a config, perhaps a command can be used to live-edit it)

When you want to go create a tx, you want to specify how many tokens 1 gas is worth. (Or perhaps how many tokens is x gas worth) So instead of Gas cost per unit, it should be tokens per unit gas.

@alexanderbez
Copy link
Contributor

alexanderbez commented Dec 13, 2018

Specified by the client/user. I think we to modify the context/action item(s) of #3118 @jackzampolin. This needs to be supplied by the client/user. Correct me if I'm wrong @ValarDragon.

@ValarDragon
Copy link
Contributor

I'm confused. What I think makes sense to do is:

  • Have a minimum fee which full nodes set for their mempool in accordance with Make antehandler local fee check more robust #3044
  • Have the client/user specify the tokens per unit gas, either via CLI or config or both. Once validators start having more sophisticated mempool validation, there should also be a tokens per p2p byte. (And eventually some means of handling storage rent)

I don't understand what the terms "Unit" or "Cost" means in Gas Cost Per Unit, hence I think tokens per unit gas is a better metric.

@alexanderbez
Copy link
Contributor

Precisely! What I’m saying is #3118 needs to address your second point.

@fedekunze
Copy link
Collaborator

FYI I changed the --fee flag on #3069 to receive sdk.Coins instead of sdk.Coin since the MinDeposit param already had that type

@alexanderbez
Copy link
Contributor

imho, this seems to be somewhat of a duplicate of #3101. Do you mind if I close @jackzampolin or are there different concerns. Users can already specify multiple fees via --fees. They just cant' configure the per unit cost of gas.

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

6 participants