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

remove latest$.gasPrice, epics and actions, calculate those directly in transact #2971

Merged
merged 3 commits into from
Oct 12, 2021

Conversation

andrevmatos
Copy link
Contributor

@andrevmatos andrevmatos commented Oct 11, 2021

Short description
technical debt tackling: since #2966 , all transactions go through transact function/utility; therefore, we don't need to keep Latest.gasPrice (kept up to date every block through its own epic), since we only need to calculate the gasPrice on that function when making a transaciton; additionally, callAndWaitMined helper used in the public Raiden class had a lot of intersection with transact's functionality, so it can be dropped for the later, allowing txs which are still directly in the public class to also benefit from the shared gasPrice and gasLimit logic there.
This shouldn't change API or UX, so no need for a changelog entry.

Definition of Done

  • Steps to manually test the change have been documented
  • Acceptance criteria are met
  • Change has been manually tested by the reviewer (dApp)

Steps to manually test the change (dApp)

blockGasPriceEpic and actions were needed only because we had calling
transactions spread across different parts of code. Now that we
centralize tx calls in `transact`, we can move gasPrice[Factor] logic to
it. This also optimizes not calling `getFeeData` on every block, and
instead just when transactioning, with a local cache to ensure
concurrent calls are deduplicated.
@github-actions
Copy link

You modified raiden-ts/src,
Please remember to add a change log entry at raiden-ts/CHANGELOG.md if necessary.

@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #2971 (c350ac1) into master (fc80222) will decrease coverage by 0.00%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2971      +/-   ##
==========================================
- Coverage   93.27%   93.26%   -0.01%     
==========================================
  Files         207      207              
  Lines        8769     8747      -22     
  Branches     1357     1353       -4     
==========================================
- Hits         8179     8158      -21     
+ Misses        489      488       -1     
  Partials      101      101              
Flag Coverage Δ
dapp 88.96% <ø> (ø)
dapp.unit 88.96% <ø> (ø)
sdk 95.81% <91.66%> (+<0.01%) ⬆️
sdk.e2e 72.88% <55.55%> (+0.07%) ⬆️
sdk.integration 80.30% <50.00%> (+0.03%) ⬆️
sdk.unit 49.44% <91.66%> (+0.68%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
raiden-ts/src/services/epics/udc.ts 100.00% <ø> (ø)
raiden-ts/src/types.ts 100.00% <ø> (ø)
raiden-ts/src/raiden.ts 94.62% <87.50%> (-0.23%) ⬇️
raiden-ts/src/channels/utils.ts 96.61% <90.00%> (-1.43%) ⬇️
raiden-ts/src/channels/actions.ts 100.00% <100.00%> (ø)
raiden-ts/src/channels/epics/block.ts 100.00% <100.00%> (ø)
raiden-ts/src/epics.ts 98.94% <100.00%> (-0.02%) ⬇️
raiden-ts/src/helpers.ts 88.44% <100.00%> (+1.17%) ⬆️
raiden-ts/src/utils/ethers.ts 91.42% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc80222...c350ac1. Read the comment docs.

This ensures txs in Raiden also use same gasPrice & gasLimit logic as epics.
An optimization to avoid having to scan since genesis, when using
`userDepositContractAddress` option instead of contracts info, and the
user knows a blockNumber old enough to catch relevant events.
@andrevmatos andrevmatos merged commit 496bea2 into master Oct 12, 2021
@andrevmatos andrevmatos deleted the refactor/gasPrice branch October 12, 2021 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants