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

Replace 'Drain' with 'Change' for clearer terminology #54

Merged
merged 9 commits into from
Oct 7, 2024

Conversation

BEULAHEVANJALIN
Copy link
Contributor

#52

/// Minimum value allowed for a drain (change) output.
pub min_drain_value: u64,
/// Minimum value allowed for a change output.
pub min_change_value: u64,

/// Strategy to use the excess value other than fee and target
pub excess_strategy: ExcessStrategy,
Copy link
Contributor

Choose a reason for hiding this comment

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

#54
Please replace ToDrain in excess strategy with ToChange

pub enum ExcessStrategy {
    ToFee,
    ToRecipient,
    ToDrain,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -672,7 +672,7 @@ fn calculate_waste(
waste += (accumulated_value - (options.target_value + estimated_fee));
Copy link
Contributor

Choose a reason for hiding this comment

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

#54 Please change this reference to ToDrain

if options.excess_strategy != ExcessStrategy::ToDrain {
        // Change is not created if excess strategy is ToFee or ToRecipient. Hence cost of change is added
        waste += (accumulated_value - (options.target_value + estimated_fee));
    } else {

src/lib.rs Outdated
Comment on lines 859 to 861
min_change_value: 500,
excess_strategy: ExcessStrategy::ToDrain,
}
Copy link
Contributor

@jkciw jkciw Oct 4, 2024

Choose a reason for hiding this comment

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

/ #54 Please change ToDrain

src/lib.rs Outdated
cost_per_input: 20,
cost_per_output: 10,
min_drain_value,
min_change_value,
excess_strategy: ExcessStrategy::ToDrain,
Copy link
Contributor

@jkciw jkciw Oct 4, 2024

Choose a reason for hiding this comment

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

#54 Please change ToDrain

src/lib.rs Outdated
cost_per_input: 20,
cost_per_output: 10,
min_drain_value: 500,
min_change_value: 500,
excess_strategy: ExcessStrategy::ToDrain,
Copy link
Contributor

Choose a reason for hiding this comment

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

#54 Please change ToDrain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback!

@delcin-raj delcin-raj merged commit 6da218c into Bitshala-Incubator:main Oct 7, 2024
8 checks passed
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