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

feat(state)!: Adding fee parameter to CoreAccessor operations #1484

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

distractedm1nd
Copy link
Collaborator

@distractedm1nd distractedm1nd commented Dec 14, 2022

Overview

Adding fee parameter to CoreAccessor operations.

All transaction endpoints now have a "fee" key needed in the JSON request. An example value for this can be 2000. This includes the following endpoints:

  • SubmitPayForData
  • Transfer
  • Delegate
  • BeginRedelegate
  • Undelegate
  • CancelUnbondingDelegation

After this change, a new field is added to the json requests in the gateway:
{oldRequest..., "fee": 2000}

@codecov-commenter
Copy link

Codecov Report

Merging #1484 (70de3df) into main (a6ac321) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

❗ Current head 70de3df differs from pull request most recent head 5983169. Consider uploading reports for the commit 5983169 to get more accurate results

@@            Coverage Diff             @@
##             main    #1484      +/-   ##
==========================================
- Coverage   55.20%   55.17%   -0.03%     
==========================================
  Files         203      203              
  Lines       12125    12135      +10     
==========================================
+ Hits         6693     6696       +3     
- Misses       4768     4776       +8     
+ Partials      664      663       -1     
Impacted Files Coverage Δ
api/gateway/state.go 0.00% <0.00%> (ø)
nodebuilder/state/mocks/api.go 11.45% <0.00%> (ø)
nodebuilder/state/state.go 7.14% <0.00%> (ø)
state/core_access.go 16.53% <0.00%> (-0.20%) ⬇️
share/ipld/get.go 92.18% <0.00%> (-1.24%) ⬇️
share/eds/byzantine/bad_encoding.go 69.00% <0.00%> (+3.00%) ⬆️
header/core/listener.go 58.49% <0.00%> (+5.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@distractedm1nd distractedm1nd self-assigned this Dec 14, 2022
@renaynay renaynay added the kind:break! Attached to breaking PRs label Dec 14, 2022
@distractedm1nd distractedm1nd marked this pull request as ready for review December 14, 2022 13:14
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Looks good - can u update public API ADR ?

Also we need to update docs repo with the following req changes

Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

LGTM, the only concern I have is bloat of params, which we can fix later by passing some of the optional params as a structure(JSON RPC lib allows that). Very low prio rn to even care, but still

@distractedm1nd distractedm1nd changed the title feat: Adding fee parameter to CoreAccessor operations feat!(state): Adding fee parameter to CoreAccessor operations Dec 14, 2022
@distractedm1nd distractedm1nd changed the title feat!(state): Adding fee parameter to CoreAccessor operations feat(state)!: Adding fee parameter to CoreAccessor operations Dec 14, 2022
@renaynay renaynay changed the title feat(state)!: Adding fee parameter to CoreAccessor operations feat!(state): Adding fee parameter to CoreAccessor operations Dec 14, 2022
@renaynay renaynay changed the title feat!(state): Adding fee parameter to CoreAccessor operations feat(state)!: Adding fee parameter to CoreAccessor operations Dec 14, 2022
@distractedm1nd distractedm1nd merged commit 4734aac into celestiaorg:main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:break! Attached to breaking PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants