diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index f8ea8c8..7343a6f 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -18,6 +18,5 @@ of the PR were done in a specific way --> #### All Submissions: -* [ ] I've signed all my commits -* [ ] I followed the [contribution guidelines](../CONTRIBUTING.md) -* [ ] I ran `make precommit` before committing \ No newline at end of file +* [ ] I followed the [contribution guidelines](https://github.com/rust-nostr/nostr/blob/master/CONTRIBUTING.md) +* [ ] I ran `just precommit` or `just check` before committing \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d705ead..9c9610a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,28 +14,20 @@ jobs: name: Format runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - run: cargo fmt --all -- --config format_code_in_doc_comments=true --check + - name: Checkout + uses: actions/checkout@v3 + - name: Install just + run: cargo install just + - name: Check + run: just check-fmt - build: - name: Build + check: + name: Check crate runs-on: ubuntu-latest steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Cache - uses: actions/cache@v3 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml','**/Cargo.lock') }} - - name: Set profile - run: rustup set profile minimal && rustup component add clippy - - name: Build - run: cargo build - - name: Tests - run: cargo test - - name: Clippy - run: cargo clippy -- -D warnings \ No newline at end of file + - name: Checkout + uses: actions/checkout@v3 + - name: Install just + run: cargo install just + - name: Check + run: just check-crate \ No newline at end of file diff --git a/.gitignore b/.gitignore index 0592392..e4039bd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ /target .DS_Store +.vscode/ +.idea/ diff --git a/CODE_STYLE.md b/CODE_STYLE.md new file mode 100644 index 0000000..2406215 --- /dev/null +++ b/CODE_STYLE.md @@ -0,0 +1,187 @@ +# Code style + +This is a description of a coding style that every contributor **must** follow. +Please, read the whole document before you start pushing code. + +## Generics + +All trait bounds should be written in `where`: + +```rust +// GOOD +pub fn new<N, T, P, E>(user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self +where + N: Into<String>, + T: Into<String>, + P: Into<InputFile>, + E: Into<String>, +{ ... } + +// BAD +pub fn new<N: Into<String>, + T: Into<String>, + P: Into<InputFile>, + E: Into<String>> + (user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self { ... } +``` + +```rust +// GOOD +impl<T> Trait for Wrap<T> +where + T: Trait +{ ... } + +// BAD +impl<T: Trait> Trait for Wrap<T> { ... } +``` + +## Use `Self` where possible + +When referring to the type for which block is implemented, prefer using `Self`, rather than the name of the type: + +```rust +impl ErrorKind { + // GOOD + fn print(&self) { + Self::Io => println!("Io"), + Self::Network => println!("Network"), + Self::Json => println!("Json"), + } + + // BAD + fn print(&self) { + ErrorKind::Io => println!("Io"), + ErrorKind::Network => println!("Network"), + ErrorKind::Json => println!("Json"), + } +} +``` + +```rust +impl<'a> AnswerCallbackQuery<'a> { + // GOOD + fn new<C>(bot: &'a Bot, callback_query_id: C) -> Self + where + C: Into<String>, + { ... } + + // BAD + fn new<C>(bot: &'a Bot, callback_query_id: C) -> AnswerCallbackQuery<'a> + where + C: Into<String>, + { ... } +} +``` + +**Rationale:** `Self` is generally shorter and it's easier to copy-paste code or rename the type. + +## Deriving traits (apply only to libraries) + +Derive `Debug`, `Clone`, `Copy`, `PartialEq`, `Eq` and `Hash` for public types when possible (in this order). + +**Rationale:** these traits can be useful for users and can be implemented for most types. + +Derive `Default` when there is a reasonable default value for the type. + +## Full paths for logging + +Always write `tracing::<op>!(...)` instead of importing `use tracing::<op>;` and invoking `<op>!(...)`. + +```rust +// GOOD +tracing::warn!("Everything is on fire"); + +// BAD +use tracing::warn; + +warn!("Everything is on fire"); +``` + +**Rationale:** +- Less polluted import blocks +- Uniformity + +## `&str` -> `String` conversion + +Prefer using `.to_string()` or `.to_owned()`, rather than `.into()`, `String::from`, etc. + +**Rationale:** uniformity, intent clarity. + +## Order of imports + +```rust +// First core, alloc and/or std +use core::fmt; +use std::{...}; + +// Second, external crates (both crates.io crates and other rust-analyzer crates). +use crate_foo::{ ... }; +use crate_bar::{ ... }; + +// If applicable, the current sub-modules +mod x; +mod y; + +// Finally, the internal crate modules and submodules +use crate::{}; +use super::{}; + +// Re-exports are treated as item definitions rather than imports, so they go +// after imports and modules. Use them sparingly. +pub use crate::x::Z; +``` + +## Import Style + +When implementing traits from `core::fmt`/`std::fmt` import the module: + +```rust +// GOOD +use core::fmt; + +impl fmt::Display for RenameError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { .. } +} + +// BAD +impl core::fmt::Display for RenameError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { .. } +} +``` + +## If-let + +Avoid the `if let ... { } else { }` construct if possible, use `match` instead: + +```rust +// GOOD +match ctx.expected_type.as_ref() { + Some(expected_type) => completion_ty == expected_type && !expected_type.is_unit(), + None => false, +} + +// BAD +if let Some(expected_type) = ctx.expected_type.as_ref() { + completion_ty == expected_type && !expected_type.is_unit() +} else { + false +} +``` + +Use `if let ... { }` when a match arm is intentionally empty: + +```rust +// GOOD +if let Some(expected_type) = this.as_ref() { + // Handle it +} + +// BAD +match this.as_ref() { + Some(expected_type) => { + // Handle it + }, + None => (), +} +``` \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bdff5a7..6e0613f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,36 +2,81 @@ ## Contribution Workflow -The codebase is maintained using the "contributor workflow" where everyone -without exception contributes patch proposals using "pull requests". This -facilitates social contribution, easy testing and peer review. +To contribute a patch: -To contribute a patch, the worflow is a as follows: +* Fork Repository +* Create topic branch +* Commit patches (PR, emails, ...) - 1. Fork Repository - 2. Create topic branch - 3. Commit patches +In general commits should be atomic and diffs **should be easy to read**. -In general commits should be atomic and diffs should be easy to read. -For this reason do not mix any formatting fixes or code moves with actual code -changes. Further, each commit, individually, should compile and pass tests, in -order to ensure git bisect and other automated tools function properly. +## Code style guide -When adding a new feature, thought must be given to the long term technical -debt. -Every new feature should be covered by unit tests where possible. +Before writing code, please read [our code style](./CODE_STYLE.md). -When refactoring, structure your PR to make it easy to review and don't -hesitate to split it into multiple small, focused PRs. +## Commit format -Commits should cover both the issue fixed and the solution's rationale. -These [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in mind. +The commit **must** be formatted in the following way: -To facilitate communication with other contributors, the project is making use -of GitHub's "assignee" field. First check that no one is assigned and then -comment suggesting that you're working on it. If someone is already assigned, -don't hesitate to ask if the assigned party or previous commenters are still -working on it if it has been awhile. +``` +<context>: <short descriptrion> + +<optional description explaining better what happened in the commit> +``` + +If applicable, link the `issue`/`PR` to be closed with: + +* Closes <url> +* Fixes <url> + +The `context` name should be: + +* `nostr` if changes are related to the main Rust `nostr` crate (or `protocol`?) +* `sdk`, `cli`, `pool`, `signer`, `nwc` and so on for the others Rust crates (so basically remove the `nostr-` prefix) +* `ffi(<name>)` for `UniFFI` and `js(<name>)` for `JavaScript` bindings (follow same above rules for the `<name>`) +* `book` if changes are related to the `book` +* `doc` for the `.md` files (except for `CHANGELOG.md`?) + +Anything that haven't a specific context, can be left without the `<context>:` prefix (ex. change to main `justfile` commands, change to `CHANGELOG.md`?) + +### Examples + +``` +nostr: add NIP32 support + +Added kinds, tags and EventBuilder constructors to support NIP32. + +Closes https://<domain>.com/rust-nostr/nostr/issue/1234 +``` + +``` +pool: fix connection issues + +Long description... + +Fixes https://<domain>.com/rust-nostr/nostr/issue/5612 +``` + +``` +nwc: add `pay_multiple_invoices` support + +Closes https://<domain>.com/rust-nostr/nostr/issue/2222 +``` + +``` +ffi(nostr): add `EventBuilder::mute_list` +``` + +``` +ffi(sdk): add `AbortHandle` + +* Return `AbortHandle` in `Client::handle_notifications` +* Another change... +``` + +``` +js(sdk): replace log `file path` with `module path` +``` ## Deprecation policy @@ -47,34 +92,10 @@ debt inside this library. If you deprecated an API as part of a contribution, we encourage you to "own" that API and send a follow-up to remove it as part of the next release cycle. -## Peer review - -Anyone may participate in peer review which is expressed by comments in the -pull request. Typically reviewers will review the code for obvious errors, as -well as test out the patch set and opine on the technical merits of the patch. -PR should be reviewed first on the conceptual level before focusing on code -style or grammar fixes. - -### Terminology +## Unwrap and expect -Concept ACK - Agree with the idea and overall direction, but haven't reviewed the code changes or tested them. - -utACK (untested ACK) - Reviewed and agree with the code changes but haven't actually tested them. - -tACK (tested ACK) - Reviewed the code changes and have verified the functionality or bug fix. - -ACK - A loose ACK can be confusing. It's best to avoid them unless it's a documentation/comment only change in which case there is nothing to test/verify; therefore the tested/untested distinction is not there. - -NACK - Disagree with the code changes/concept. Should be accompanied by an explanation. +Usage of `.unwrap()` or `.expect("...")` methods is allowed **only** in `examples` or `tests`. ## Coding Conventions -Use `cargo fmt` and `cargo clippy` with the default settings to format code before committing. -This is also enforced by the CI. - -## Going further - -You may be interested by Jon Atacks guide on [How to review Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-review-bitcoin-core-prs.md) -and [How to make Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-make-bitcoin-core-prs.md). -While there are differences between the projects in terms of context and -maturity, many of the suggestions offered apply to this project. \ No newline at end of file +Use `just precommit` or `just check` to format and check the code before committing. This is also enforced by the CI. diff --git a/Makefile b/Makefile deleted file mode 100644 index 84cf8bf..0000000 --- a/Makefile +++ /dev/null @@ -1,27 +0,0 @@ -# Use 'VERBOSE=1' to echo all commands, for example 'make help VERBOSE=1'. -ifdef VERBOSE - Q := -else - Q := @ -endif - -all: build - -help: - $(Q)echo "" - $(Q)echo "make build - Build binary" - $(Q)echo "make precommit - Execute precommit steps" - $(Q)echo "make loc - Count lines of code in src folder" - $(Q)echo "" - -build: - $(Q)cargo build --release - -precommit: - $(Q)cargo fmt && cargo clippy - -clean: - $(Q)cargo clean - -loc: - $(Q)echo "--- Counting lines of .rs files in 'src' (LOC):" && find src/ -type f -name "*.rs" -exec cat {} \; | wc -l diff --git a/contrib/scripts/check-crate.sh b/contrib/scripts/check-crate.sh new file mode 100755 index 0000000..43fd4df --- /dev/null +++ b/contrib/scripts/check-crate.sh @@ -0,0 +1,7 @@ +#!/bin/bash + +set -euo pipefail + +cargo check +cargo test +cargo clippy -- -D warnings \ No newline at end of file diff --git a/contrib/scripts/check-fmt.sh b/contrib/scripts/check-fmt.sh new file mode 100755 index 0000000..6f9874c --- /dev/null +++ b/contrib/scripts/check-fmt.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +set -euo pipefail + +flags="" + +# Check if "check" is passed as an argument +if [[ "$#" -gt 0 && "$1" == "check" ]]; then + flags="--check" +fi + +# Install toolchain +rustup install nightly-2024-01-11 +rustup component add rustfmt --toolchain nightly-2024-01-11 + +# Check workspace crates +cargo +nightly-2024-01-11 fmt --all -- --config format_code_in_doc_comments=true $flags \ No newline at end of file diff --git a/doc/ndk-rest.service b/doc/nostr-rest.service similarity index 81% rename from doc/ndk-rest.service rename to doc/nostr-rest.service index de5fee7..40b8518 100644 --- a/doc/ndk-rest.service +++ b/doc/nostr-rest.service @@ -1,8 +1,8 @@ [Unit] -Description=NDK Rest API +Description=Nostr Rest API [Service] -ExecStart=/usr/local/bin/ndk-rest +ExecStart=/usr/local/bin/nostr-rest User=<yourusername> Type=simple KillMode=process diff --git a/justfile b/justfile new file mode 100644 index 0000000..6085074 --- /dev/null +++ b/justfile @@ -0,0 +1,37 @@ +default: build + +# Build (release) +build: + cargo build --release + +bench: + ab -n 100000 -c 16 -k -r -t 60 http://127.0.0.1:7773/ping + +# Check format and crates +check: check-fmt check-crate + +# Format the code and execute some checks +precommit: fmt + cargo check + cargo test + cargo clippy + +# Format the entire Rust code +fmt: + @bash contrib/scripts/check-fmt.sh + +# Check if the Rust code is formatted +check-fmt: + @bash contrib/scripts/check-fmt.sh check + +# Check crate +check-crate: + @bash contrib/scripts/check-crate.sh + +# Remove artifacts that cargo has generated +clean: + cargo clean + +# Count the lines of codes of this project +loc: + @echo "--- Counting lines of .rs files (LOC):" && find crates/ bindings/ -type f -name "*.rs" -not -path "*/target/*" -exec cat {} \; | wc -l diff --git a/rustfmt.toml b/rustfmt.toml index c1438be..beeb781 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -2,10 +2,8 @@ tab_spaces = 4 newline_style = "Auto" reorder_imports = true reorder_modules = true - -# nightly (to enable when are stable) -# indent_style = "Block" -# normalize_comments = true -# imports_granularity = "Module" -# reorder_impl_items = true -# group_imports = "StdExternalCrate" \ No newline at end of file +reorder_impl_items = false +indent_style = "Block" +normalize_comments = false +imports_granularity = "Module" +group_imports = "StdExternalCrate" \ No newline at end of file diff --git a/src/config/mod.rs b/src/config/mod.rs index 917059c..8c30402 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,10 +1,10 @@ // Copyright (c) 2023 Nostr Development Kit Devs // Distributed under the MIT software license +use std::fs; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::path::PathBuf; use std::str::FromStr; -use std::fs; use clap::Parser; use nostr_sdk::Url; @@ -15,7 +15,6 @@ pub mod model; pub use self::model::Config; use self::model::{ConfigFile, Limit, Network, Nostr, Redis}; - fn default_dir() -> PathBuf { let home: PathBuf = dirs::home_dir().unwrap_or_else(|| { panic!("Unknown home directory"); @@ -52,7 +51,7 @@ impl Config { pub fn get() -> Self { let args: Args = Args::parse(); - let config_file_path: PathBuf = args.config.unwrap_or_else(|| default_config_file()); + let config_file_path: PathBuf = args.config.unwrap_or_else(default_config_file); let content = fs::read_to_string(config_file_path).expect("Impossible to read config file"); let config_file: ConfigFile = toml::from_str(&content).expect("Impossible to parse config file"); diff --git a/src/config/model.rs b/src/config/model.rs index 79cbdc3..b770282 100644 --- a/src/config/model.rs +++ b/src/config/model.rs @@ -4,9 +4,8 @@ use std::fmt; use std::net::SocketAddr; -use serde::Deserialize; - use nostr_sdk::Url; +use serde::Deserialize; #[derive(Debug, Clone)] pub struct Network { diff --git a/src/error.rs b/src/error.rs index c98c209..6be02c9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,10 +1,9 @@ // Copyright (c) 2023 Nostr Development Kit Devs // Distributed under the MIT software license -use axum::{ - extract::{rejection::JsonRejection, FromRequest}, - response::{IntoResponse, Response}, -}; +use axum::extract::rejection::JsonRejection; +use axum::extract::FromRequest; +use axum::response::{IntoResponse, Response}; use serde_json::json; #[derive(FromRequest)] diff --git a/src/handler.rs b/src/handler.rs index fb8d580..f489feb 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -1,7 +1,8 @@ // Copyright (c) 2023 Nostr Development Kit Devs // Distributed under the MIT software license -use axum::{extract::State, response::Json}; +use axum::extract::State; +use axum::response::Json; use nostr_sdk::hashes::sha256::Hash as Sha256Hash; use nostr_sdk::hashes::Hash; use nostr_sdk::{Event, Filter}; diff --git a/src/main.rs b/src/main.rs index 2088700..eca70a6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,14 +1,13 @@ // Copyright (c) 2023 Nostr Development Kit Devs // Distributed under the MIT software license -use axum::{ - http::Method, - routing::{get, post}, - Router, -}; +use std::time::Duration; + +use axum::http::Method; +use axum::routing::{get, post}; +use axum::Router; use nostr_sdk::{Client, Keys, Result}; use redis::Client as RedisClient; -use std::time::Duration; use tower_http::cors::{Any, CorsLayer}; use tower_http::trace::TraceLayer;