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

Estimate fee Api endpoint response taking ~10 seconds #1825

Closed
nikolaglumac opened this issue Jun 29, 2020 · 3 comments
Closed

Estimate fee Api endpoint response taking ~10 seconds #1825

nikolaglumac opened this issue Jun 29, 2020 · 3 comments
Assignees
@rvl

Comments

@nikolaglumac
Copy link

nikolaglumac commented Jun 29, 2020 •
edited by rvl
Loading

#1569
Context

There is a regression in the performance of the "Estimate fee" Api endpoint (https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/postTransactionFee) for which the response time has been increased from ~100 milliseconds to ~10 seconds. This happens regardless of the wallet's UTXO fragmentation (it is the same for wallets with a single UTXO as for the ones with >100 UTXOs) and is reproducible every time.
Please note that there is no impact on the "Delegation fee estimation" Api endpoint (https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/getDelegationFee) which has almost instant response...

Logs: logs.zip

Information -
Version f6d5a80 (master branch)
Platform All
Installation Daedalus, develop build

Steps to Reproduce

  1. Restore a wallet with funds (or send funds to an empty wallet)
  2. Proceed to "Send" screen in Daedalus
  3. Enter valid receiving address (it can be an address from the same wallet)
  4. Enter valid ada amount
  5. Fee estimation endpoint request is triggered upon entering the amount
  6. Fees will be visible only after ~10 seconds.

Expected behavior

Fees should be visible almost instantly.

Actual behavior

Fees are visible only after ~10 seconds.

Resolution

  1. Make shelley transaction size calculation faster ⇒ Make shelley fee estimation faster #1834.
  2. Set up shelley versions of API response time benchmarks, to detect performance issues earlier ⇒ Begin porting latency benchmark to shelley #1829 Add latency benchmark for shelley #1845.

QA

  1. Existing tests check that results are the same as before.
  2. A new benchmark for the shelley API runs in the nightly tests.
@rvl rvl self-assigned this Jun 30, 2020
@rvl
Copy link
Contributor

rvl commented Jun 30, 2020 •
edited
Loading

I did some more testing - even estimating transaction fees on a completely empty wallet is slow! (it takes 10 seconds to say there is not enough ada)

  • Interestingly, on the cardano-wallet-byron backend, fee estimation is fast. On both byron and shelley, the estimation is run 100 times and statistics gathered.

    We only have a nightly benchmark running for byron, not shelley. These byron benchmarks are showing reasonable fee estimation performance for wallets with up to 1000 UTxO.

  • Delegation fee estimation is not slow, because this does not require coin selection. That is the part that is running slowly for shelley.

  • I have checked the cabal and nix files to ensure that cardano-wallet-shelley is built with the same optimisation level as cardano-wallet-byron.

Logs timing the fee estimation:

This was referenced Jun 30, 2020
Merged
Merged
iohk-bors bot added a commit that referenced this issue Jun 30, 2020
@iohk-bors @rvl
Merge #1834
10abd51 
1834: Make shelley fee estimation faster r=rvl a=rvl

Relates to #1825.

### Overview

This removes the crypto signature operations from the function which calculates a transaction's size.
    
It reduces fee estimation (x100 runs) time from 7.98s to 1.66s.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Jun 30, 2020
@iohk-bors @rvl
Merge #1834
e71dd1a 
1834: Make shelley fee estimation faster r=rvl a=rvl

Relates to #1825.

### Overview

This removes the crypto signature operations from the function which calculates a transaction's size.
    
It reduces fee estimation (x100 runs) time from 7.98s to 1.66s.


Co-authored-by: Rodney Lorrimar <[email protected]>
@rvl rvl mentioned this issue Jul 2, 2020
Merged
@piotr-iohk
Copy link
Contributor

Manually testing on shelley testnet it takes ~1-2 seconds for fees to be returned.

@piotr-iohk
Copy link
Contributor

LGTM. Latency benchmarks for shelley, which were sort of introduced along with the fix, are still to be merged.

iohk-bors bot added a commit that referenced this issue Jul 7, 2020
@iohk-bors @rvl
Merge #1829
e855495 
1829: Begin porting latency benchmark to shelley r=piotr-iohk a=rvl

Relates to #1825

# Overview

These refactors are the start of porting the latency bench from the byron backend to the shelley backend.

The measurement and reporting code is split into a separate module.

The bench scenario functions are reworked so that swapping backend will be easier.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 8, 2020
@iohk-bors @rvl
Merge #1845
60c6973 
1845: Add latency benchmark for shelley r=rvl a=rvl

Relates to #1825.

### Overview

- Ports latency benchmarks to shelley backend.
- Refactors duplicate code into a shared module.
- Adds listStakePools to benchmarks

Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 13, 2020
@iohk-bors @rvl @jonathanknowles
Merge #1845 #1899
8d2f114 
1845: Add latency benchmark for shelley r=rvl a=rvl

Relates to #1825.

### Overview

- Ports latency benchmarks to shelley backend.
- Refactors duplicate code into a shared module.
- Adds listStakePools to benchmarks

1899: Enable `STAKE_POOLS_LIST` tests to run in isolation. r=jonathanknowles a=jonathanknowles

# Issue Number

#1819 

# Overview

The very first test in the `STAKE_POOLS_LIST_01` series has implicit dependency on time. If run _too soon_ (as is the case if the test is run in isolation), such that the wallet has not yet had a chance to catch up with tip, then the wallet API will return an empty list, which will cause a test failure.

To fix this, this PR nests the test inside an `eventually` combinator, which allows the test to be run in isolation (just like the others in the `STAKE_POOLS_LIST_01` series).

Additional changes:

- [x] Fixes some spelling errors.
- [x] Reformatting to comply with our coding standards.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 13, 2020
@iohk-bors @rvl
Merge #1845
a441a4c 
1845: Add latency benchmark for shelley r=rvl a=rvl

Relates to #1825.

### Overview

- Ports latency benchmarks to shelley backend.
- Refactors duplicate code into a shared module.
- Adds listStakePools to benchmarks

Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 14, 2020
@iohk-bors @rvl
Merge #1845
bada112 
1845: Add latency benchmark for shelley r=rvl a=rvl

Relates to #1825.

### Overview

- Ports latency benchmarks to shelley backend.
- Refactors duplicate code into a shared module.
- Adds listStakePools to benchmarks

Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 14, 2020
@iohk-bors @rvl
Merge #1845
52966c3 
1845: Add latency benchmark for shelley r=rvl a=rvl

Relates to #1825.

### Overview

- Ports latency benchmarks to shelley backend.
- Refactors duplicate code into a shared module.
- Adds listStakePools to benchmarks

Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rvl rvl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nikolaglumac @rvl @piotr-iohk