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

Add estimate fee endpoint #507

Merged
merged 6 commits into from
Jul 4, 2019
Merged

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Jul 4, 2019

Issue Number

#469

Overview

  • I have added implementation of estimateFee endpoint
  • I have added respective typesSpec unit tests
  • I have added corresponding integration tests unifying them with already present ones

Comments

@paweljakubas paweljakubas requested a review from KtorZ July 4, 2019 10:53
@paweljakubas paweljakubas self-assigned this Jul 4, 2019
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

Awesome work @paweljakubas !
I haven't look in depth to the integration scenarios. From a quick glance, this looks okay, but I'll spend a bit more time later reviewing carefully and I invite @piotr-iohk to also do the same 😄

lib/core/src/Cardano/Wallet.hs Outdated Show resolved Hide resolved
specifications/api/swagger.yaml Show resolved Hide resolved
lib/core/src/Cardano/Wallet.hs Outdated Show resolved Hide resolved
Copy link
Contributor

@piotr-iohk piotr-iohk left a comment

Choose a reason for hiding this comment

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

I've reviewed the tests - looking good, thanks a lot @paweljakubas ! :)
There's one test that could be added: estimate transaction for amount = 0. That one should behave different for http-bridge and jormungandr so they should go to their specific libs, like:

http-bridge -> https://github.com/input-output-hk/cardano-wallet/blob/master/lib/http-bridge/test/integration/Test/Integration/HttpBridge/Scenario/API/Transactions.hs

jormungandr -> https://github.com/input-output-hk/cardano-wallet/tree/master/lib/jormungandr/test/integration/Test/Integration/Jormungandr/Scenario/

}|]
r <- request @ApiFee ctx (postTxFeeEp wSrc) Default payload
verify r
[ expectResponseCode HTTP.status202 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps let's also check the amount here: expectFieldBetween amount...

lib/core/src/Cardano/Wallet.hs Show resolved Hide resolved
@KtorZ KtorZ merged commit 2951956 into master Jul 4, 2019
@KtorZ KtorZ deleted the paweljakubas/469/add-estimate-fee-endpoint branch July 4, 2019 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants