Merge pull request #167 from enigbe/chore-consistent-formating-across-repo

chore: add rustfmt and clippy

Author: carlaKC

.github/workflows/build-and-test.yml

@@ -6,19 +6,16 @@ permissions:
   contents: read
 
 jobs:
-  clippy:
-    name: Lint
+  check:
+    name: Check Code with stable output
     runs-on: ubuntu-latest
     steps:
+      - name: Install protoc
+        run: sudo apt install -y protobuf-compiler
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
-        with:
-          components: clippy
-      - uses: giraffate/clippy-action@v1
-        with:
-          reporter: github-pr-review
-          github_token: ${{ secrets.GITHUB_TOKEN }}
-          clippy_flags: --all-features  --all-targets --color always -- --deny warnings
+      - name: run check
+        run: make check
 
   build:
     name: Build and test

CONTRIBUTING.md

@@ -0,0 +1,37 @@
+## Contributing 
+
+SimLN is open and welcomes contributions from the community. A good place to start if you have questions is our [discussions forum](https://github.com/bitcoin-dev-project/sim-ln/discussions/123). 
+
+## Pull Request (PR) Requirements
+
+The list below contains mandatory checks to be adhered to before a PR can be opened and/or reviewed. 
+
+1. [Testing](#testing)
+2. [Formating and Linting](#formatting-and-linting)
+4. [Commit history](#commit-history)
+
+**Note**: Reviews are contigent on these checks passing locally and in CI.
+
+### Testing
+
+All tests must pass. Do so by running the command
+
+```sh
+$ cargo test
+```
+
+### Formatting and Linting
+
+Formatting and linting should adhere to the rules specified by default and/or specifically in `rustfmt.toml`. Adhere to the linter suggestions made by `clippy`.
+In addition to these check, the recipe `stable-output` ensures consistent and reliable output locally and in CI environment.
+
+```sh
+$ make check
+```
+
+### Commit history
+
+- Have a clean commit history: It is preferable to use [fixup and squash](https://andrewlock.net/smoother-rebases-with-auto-squashing-git-commits/) in addressing feedback from PR review. The former (i.e. `fixup`) for the commits they apply to (during review), and latter (`squash`) once review is complete.
+- Use [good commit messages](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html)
+- Resolve all conflicts
+- Rebase often

Makefile

@@ -31,4 +31,18 @@ run-interactive:
 	docker run -it --rm --name sim-ln --init -v simln-data:${DEFAULT_DATA_DIR} -e SIMFILE_PATH=${DEFAULT_SIMFILE_PATH} -e DATA_DIR=${DATA_DIR} -e LOG_LEVEL=$(LOG_LEVEL) -e HELP=${HELP} -e PRINT_BATCH_SIZE=${PRINT_BATCH_SIZE} -e TOTAL_TIME=${TOTAL_TIME} sim-ln
 
 stop-docker:
-	docker stop sim-ln
+	docker stop sim-ln
+
+check-code:
+	cargo fmt --check --all
+	cargo clippy --all-features
+
+stable-output:
+	@if [ -n "$$(git status --porcelain)" ]; then \
+    	echo "Error: There are unstaged or uncommitted changes after running 'make check-code'."; \
+    	exit 1; \
+	else \
+		echo "No unstaged or uncommitted changes found."; \
+	fi
+
+check: check-code stable-output

README.md

@@ -49,6 +49,8 @@ sim-cli
 
 Run `sim-cli -h` for details on `--data-dir` and `--sim-file` options that allow specifying custom simulation file locations.
 
+Interested in contributing to the project? See [CONTRIBUTING](CONTRIBUTING.md) for more details.
+
 ### Simulation File Setup
 The simulator requires access details for a set of `nodes` that you 
 have permission to execute commands on. Note that the current version 
@@ -207,4 +209,4 @@ see the [Scaling Lightning](https://github.com/scaling-lightning/scaling-lightni
 project.
 
 ## Docker
-If you want to run the cli in a containerized environment, see the docker set up docs [here](./docker/README.md)
+If you want to run the cli in a containerized environment, see the docker set up docs [here](./docker/README.md)

rustfmt.toml

@@ -0,0 +1,4 @@
+edition = "2021"
+match_block_trailing_comma = true
+newline_style = "Unix"
+use_try_shorthand = true

sim-cli/src/main.rs

@@ -40,7 +40,7 @@ fn deserialize_f64_greater_than_zero(x: String) -> Result<f64, String> {
                     "capacity_multiplier must be higher than 0. {x} received."
                 ))
             }
-        }
+        },
         Err(e) => Err(e.to_string()),
     }
 }
@@ -159,7 +159,7 @@ async fn main() -> anyhow::Result<()> {
                         act.destination
                     )));
                 }
-            }
+            },
             NodeId::PublicKey(pk) => {
                 if let Some(info) = pk_node_map.get(pk) {
                     info.clone()
@@ -179,7 +179,7 @@ async fn main() -> anyhow::Result<()> {
                             ))
                         })?
                 }
-            }
+            },
         };
 
         validated_activities.push(ActivityDefinition {

sim-lib/src/lib.rs

@@ -51,13 +51,13 @@ impl NodeId {
                     return Err(LightningError::ValidationError(format!(
                     "the provided node id does not match the one returned by the backend ({} != {}).", pk, node_id)));
                 }
-            }
+            },
             crate::NodeId::Alias(a) => {
                 if alias != a {
                     log::warn!("The provided alias does not match the one returned by the backend ({} != {}).", a, alias)
                 }
                 *alias = a.to_string();
-            }
+            },
         }
         Ok(())
     }
@@ -752,7 +752,7 @@ async fn consume_events(
                         // We need to track the payment outcome using the payment hash that we have received.
                         payment.hash = Some(payment_hash);
                         SimulationOutput::SendPaymentSuccess(payment)
-                    }
+                    },
                     Err(e) => {
                         log::error!(
                             "Error while sending payment {} -> {}.",
@@ -765,23 +765,23 @@ async fn consume_events(
                                 log::error!("Simulation terminated with error: {s}.");
                                 shutdown.trigger();
                                 break;
-                            }
+                            },
                             _ => SimulationOutput::SendPaymentFailure(
                                 payment,
                                 PaymentResult::not_dispatched(),
                             ),
                         }
-                    }
+                    },
                 };
 
                 match sender.send(outcome).await {
-                    Ok(_) => {}
+                    Ok(_) => {},
                     Err(e) => {
                         log::error!("Error sending action outcome: {:?}.", e);
                         break;
-                    }
+                    },
                 }
-            }
+            },
         };
     }
 }
@@ -1055,20 +1055,20 @@ async fn track_payment_result(
                         res.payment_outcome
                     );
                     res
-                }
+                },
                 Err(e) => {
                     log::error!("Track payment failed for {}: {e}.", hex::encode(hash.0));
                     PaymentResult::track_payment_failed()
-                }
+                },
             }
-        }
+        },
         // None means that the payment was not dispatched, so we cannot track it.
         None => {
             log::error!(
                 "We cannot track a payment that has not been dispatched. Missing payment hash."
             );
             PaymentResult::not_dispatched()
-        }
+        },
     };
 
     if results.clone().send((payment, res)).await.is_err() {

sim-lib/src/lnd.rs

@@ -127,7 +127,7 @@ impl LightningNode for LndNode {
                 return Err(LightningError::ValidationError(
                     "simnet is not supported".to_string(),
                 ))
-            }
+            },
             x => x,
         })
         .map_err(|err| LightningError::ValidationError(err.to_string()))?)