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

Getting minimum utxo #1894

Merged
merged 8 commits into from Jul 10, 2020
Merged

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Jul 10, 2020

Issue Number

#1893

Overview

  • I have extended ProtocolParameters and ApiNetworkParameters
  • I have updated swagger
  • I have used minimumUtxoValue in feeOpts in determination of dustThreshold
  • I have updated testing
  • I have handled migration

Comments

@paweljakubas paweljakubas added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Jul 10, 2020
@paweljakubas paweljakubas requested a review from KtorZ July 10, 2020 11:57
@paweljakubas paweljakubas self-assigned this Jul 10, 2020
@paweljakubas
Copy link
Contributor Author

bors try

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

let feePolicy@(LinearFee (Quantity a) _ _) = txp ^. #getFeePolicy
let feeOptions = (feeOpts tl Nothing feePolicy)
{ dustThreshold = Coin $ ceiling a }
let minUtxo' = max (Coin $ ceiling a) minUtxo
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -152,6 +152,7 @@ ProtocolParameters
protocolParametersTxMaxSize Word16 sql=tx_max_size
protocolParametersDecentralizationLevel Percentage sql=decentralization_level
protocolParametersDesiredNumberOfPools Word16 sql=desired_pool_number
protocolParametersMinimumUtxoValue W.Coin sql=minimum_utxo_value
Copy link
Member

Choose a reason for hiding this comment

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

We're going to need a migration for this one with a default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cherry-picked one commit and also handled the migration in 487bb4f

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.

Looks good as a first step, I wonder how well it's going to work with the coin selection.

Beware that we need a migration for the added column!

paweljakubas and others added 7 commits July 10, 2020 15:22
Turns out that adding column is a pretty common and straighfordward migration. I originally intended to add a new column to the TxMeta table, hence the refactor. In the end, I've used a different table for withdrawals, but the refactor is still worth it as it is IMO.
@paweljakubas paweljakubas force-pushed the paweljakubas/1893/getting-minimum-utxo branch from 487bb4f to ac66d37 Compare July 10, 2020 13:22
@paweljakubas paweljakubas requested a review from KtorZ July 10, 2020 13:26
adjust the rest of codebase
@paweljakubas paweljakubas force-pushed the paweljakubas/1893/getting-minimum-utxo branch from ac66d37 to 193adf3 Compare July 10, 2020 13:35
@paweljakubas
Copy link
Contributor Author

bors try

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

try

Build failed

@paweljakubas
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

This PR was included in a batch with a merge conflict, it will be automatically retried

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

@iohk-bors iohk-bors bot merged commit 749c243 into master Jul 10, 2020
@iohk-bors iohk-bors bot deleted the paweljakubas/1893/getting-minimum-utxo branch July 10, 2020 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants