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

Non-zero Default Fees #9106

Closed
Tracked by #9572
aaronc opened this issue Apr 13, 2021 · 6 comments · Fixed by #9371
Closed
Tracked by #9572

Non-zero Default Fees #9106

aaronc opened this issue Apr 13, 2021 · 6 comments · Fixed by #9371

Comments

@aaronc
Copy link
Member

aaronc commented Apr 13, 2021

As a short term solution to #8224, we have decided to make a change to how the minimum-gas-prices config parameter is processed.

There are a couple of ways this could work specifically:

  1. by default make it an error for validators to set minimum-gas-prices to an empty value. This will require validators to set a minimum fee OR explicitly set their fees to zero, or
  2. force app developers to set a default value for minimum-gas-prices which will be used if validators leave this param empty

Adding to the feature backlog for the next release.

@tac0turtle
Copy link
Member

This seems like its already possible (https://github.com/cosmos/cosmos-sdk/blob/master/simapp/simd/cmd/root.go#L205) but it's not documented and everyone copies the simd implementation in the sdk.

Also it seems we set minGasPrice to "" and this defaults to 0, which could be considered a bug.

@MikeSofaer
Copy link

My chain (Pylons) is primarily a 0-fee chain, it's modeled on free-to-play games, where we want to get users engaged and playing before we start charging them money. Similarly for artists we want them to be able to mint pieces without paying up-front at all. I have other strategies for spam prevention. So I'd like to make sure that 0-fee remains possible, please!

@clevinson
Copy link
Contributor

After discussing on the SDK Sprint Planning call today, the desired behavior we're wanting here is to have:

  • server will error if there's an empty value for min-gas-price
  • we will document how an app developer can set a default (to avoid all nodes from erroring if they leave it empty)

@robert-zaremba
Copy link
Collaborator

Linking a more general solution we discussed for cosmos, and we are proposing to implement in Regen for chain minimum gas price (gov controlled lower bound for validators' minimum_gas_price).

@hxrts
Copy link
Contributor

hxrts commented Jun 7, 2021

This does raise a question of whether we want this to land in the SDK as a default solution. Any of the min-fee approaches discussed previously would likely supersede this feature. Long-term I think want the fee system to act more like modules where devs elect to add an encapsulated solution that works for their chain. Maybe it's useful to include in the short term though?

@robert-zaremba
Copy link
Collaborator

Yes - we were always talking about a need for the short term solution. Maybe it could be a secondary / optional in the future? Nobody is working on dynamic fee mechanism (we need to spec the mechanism which will work with tx prioritization epic). We are only hoping that the Terra story won't repeat with any other chain. Note that it's enough to have a sabotage validator to cause the spam attack.

As we discussed during the calls, we can't follow Ethereum 1.0 examples here, because Ethereum miners have much bigger cost to produce a block than Cosmos validators. And there is not much spam a sabotage miner can cause (max 15 spam tx / sec with aforementioned big mining cost).

@mergify mergify bot closed this as completed in #9371 Jun 25, 2021
mergify bot pushed a commit that referenced this issue Jun 25, 2021
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review.
-->

closes: #9106

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes
mergify bot pushed a commit that referenced this issue Jun 25, 2021
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review.
-->

closes: #9106

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 3fd376b)

# Conflicts:
#	CHANGELOG.md
#	contrib/rosetta/node/data.tar.gz
#	docs/core/cli.md
#	server/util_test.go
#	simapp/simd/cmd/root.go
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this issue May 22, 2023
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review.
-->

closes: cosmos#9106

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants