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

Validate zero amount in a defined activity #180

Closed
wants to merge 2 commits into from
Closed

Validate zero amount in a defined activity #180

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
127c850
return validation error if the amount_msat in activity is zero
joseguru Apr 9, 2024
af5ae73
fixed lint issue. Moved validation of amount_msat to after the source…
joseguru Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions sim-lib/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,12 @@ impl Simulation {
}

for payment_flow in self.activity.iter() {
// lets validate the amount_msats to be greater than zero
if payment_flow.amount_msat==0 {
Copy link

Choose a reason for hiding this comment

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

This needs proper formatting. The contributing guide recommends running a make check command to check for lint errors and adhering to the linter suggestions made by clippy.

Copy link

Choose a reason for hiding this comment

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

nit: I think having the zero amount validation after the source and destination nodes validation makes more sense because the payment amount_msat is coming from a valid source node to a valid destination node.

Copy link
Author

Choose a reason for hiding this comment

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

@Anyitechs. I see. This makes sense

return Err(LightningError::ValidationError(
"Expected amount_msat should be greater than zero.".to_string(),
Copy link

Choose a reason for hiding this comment

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

I think the error message here can be more descriptive, because some users may find this hard to understand at first.

Suggested change
"Expected amount_msat should be greater than zero.".to_string(),
""We do not allow defined activity amount_msat with zero values".to_string(),

));
}
// We need every source node that is configured to execute some activity to be included in our set of
// nodes so that we can execute events on it.
self.nodes
Expand Down
Loading