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.

Sign up for GitHub

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

Jump to bottom

feat(app): set up post handler for tx tips #1454

Closed
wants to merge 2 commits into from
Closed

feat(app): set up post handler for tx tips #1454

wants to merge 2 commits into from

Conversation

technicallyty
Copy link
Contributor

Description

enables tx tips on the application

Closes: #1331

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@technicallyty technicallyty marked this pull request as ready for review August 31, 2022 22:10
@technicallyty technicallyty requested a review from a team August 31, 2022 22:10
@codecov
Copy link

codecov bot commented Aug 31, 2022

Codecov Report

Merging #1454 (90b279a) into main (4ef8001) will decrease coverage by 0.00%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main    #1454      +/-   ##
==========================================
- Coverage   78.48%   78.47%   -0.01%     
==========================================
  Files         238      238              
  Lines       18439    18447       +8     
==========================================
+ Hits        14471    14477       +6     
- Misses       3126     3127       +1     
- Partials      842      843       +1
Impacted Files Coverage Δ
app/stable_appconfig.go 44.18% <71.42%> (+5.29%) ⬆️
app/app.go 92.70% <100.00%> (+0.01%) ⬆️

ryanchristo
ryanchristo requested changes Aug 31, 2022
View reviewed changes
Copy link
Member

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should first identify potential use cases and then consider including in the next release alongside other v0.46 updates

@ryanchristo ryanchristo changed the title feat(app)!: setup tx tips feat(app)!: set up tx tips Aug 31, 2022
@ryanchristo
Copy link
Member

Would be great if you could do a little research on how we might use transaction tips before we implement this.

@technicallyty
Copy link
Contributor Author

Would be great if you could do a little research on how we might use transaction tips before we implement this.

  • user receives ecocredits from send/create_batch/bridge but has no REGEN balance, can then move the credits around w/o having to acquire REGEN (keep in mind, REGEN will still cycle through the system, its just paid for by the fee payer/relayer in exchange for whatever the user's tip is)
  • the ol' "staked all my coins but now i can't claim my rewards" problem can be fixed w/ tips 😺
  • validator w/ extra liquid REGEN can setup a market for tips allowing them to earn other IBC coins for regen ledger transactions

also fair warning, we may want to keep this blocked for now as it is technically still beta software. may want to consider waiting till full release

@ryanchristo ryanchristo changed the title feat(app)!: set up tx tips feat(app)!: set up post handler for tx tips Aug 31, 2022
@ryanchristo ryanchristo changed the title feat(app)!: set up post handler for tx tips feat(app): set up post handler for tx tips Sep 16, 2022
@ryanchristo
Copy link
Member

On hold until we have automated fee payer infrastructure in place. Until then, using the tips field will error rather than no error with no automated fee payer infrastructure in place.

@ryanchristo
Copy link
Member

Closing this as we are not planning to implement a fee payer infrastructure and we are considering an alternative (allowing for multiple fees - another solution that chains are using). We can re-open and revisit if we change our mind.

@ryanchristo ryanchristo closed this Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanchristo ryanchristo ryanchristo requested changes

Assignees
No one assigned
Labels
Status: Blocked
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add post handler to support transaction tips
2 participants
@technicallyty @ryanchristo