|
| 1 | +# Contributing to `Krux` firmware |
| 2 | + |
| 3 | +The development of `Krux` firmware is FOSS and community-effort-based and |
| 4 | +welcomes contributions from anyone. We are excited you are interested in |
| 5 | +helping us bring sovereign and private self-custody to everyone. |
| 6 | + |
| 7 | +We welcome contributions in many forms, including bug reports, feature |
| 8 | +requests, code contributions, and documentation improvements, from any |
| 9 | +contributors with any level of experience or expertise. |
| 10 | + |
| 11 | +Besides that, we ask that you respect others and follow the process |
| 12 | +outlined in this document. |
| 13 | + |
| 14 | +--- |
| 15 | + |
| 16 | +## Read the docs carefully |
| 17 | + |
| 18 | +We have a deep and a concise |
| 19 | +[documentation for users](https://selfcustody.github.io/krux). Besides |
| 20 | +that, not all of them read the documentation properly. |
| 21 | + |
| 22 | +As a developer, don't do that. Read carefully the current documentation |
| 23 | +and, if you think it isn't sufficient for the utmost standards, please |
| 24 | +[make a PR](https://github.com/selfcustody/krux/pulls/new); it's a very |
| 25 | +`good first issue` and a welcomed contribution. |
| 26 | + |
| 27 | +--- |
| 28 | + |
| 29 | +## Non `Krux` firmware contributions |
| 30 | + |
| 31 | +The `Krux` growth in the Bitcoin ecosystem is due to developments beyond firmware. |
| 32 | +They're built from real, material, and daily use cases from our beloved |
| 33 | +community of kruxers. While we love that, we cannot be responsible for any |
| 34 | +development made by our users. |
| 35 | + |
| 36 | +Our developers and contributors are listed in both [krux contributors list](https://github.com/selfcustody/krux/contributors) |
| 37 | +and [krux-installer contributors list](https://github.com/selfcustody/krux-installer/contributors) |
| 38 | +. |
| 39 | + |
| 40 | +--- |
| 41 | + |
| 42 | +## Communications Channels |
| 43 | + |
| 44 | +The primary communication channel is the [GitHub repository](https://github.com/selfcustody/krux) |
| 45 | +. |
| 46 | + |
| 47 | +--- |
| 48 | + |
| 49 | +## Contribution Workflow |
| 50 | + |
| 51 | +The contribution workflow is designed to facilitate cooperation and ensure a |
| 52 | +high level of quality in the project and not to enforce any type of dogmatic |
| 53 | +procedure on creative level. The process is as follows: |
| 54 | + |
| 55 | +To contribute a patch, the workflow is as follows: |
| 56 | + |
| 57 | + 1. Fork Repository |
| 58 | + 2. Create topic branch |
| 59 | + 3. Commit patches |
| 60 | + |
| 61 | +### Fork Repository |
| 62 | + |
| 63 | +To fork a repository you need a basic understanding of how git and GitHub works. |
| 64 | +Basically you will need to [fork the repository](https://github.com/selfcustody/krux), |
| 65 | +then clone it locally on your computer and set both upstream and your repo. |
| 66 | +It's recommended that you use `ssh` connections instead of `https` ones: |
| 67 | + |
| 68 | +```bash |
| 69 | +# after fork, you could clone via https: |
| 70 | +git clone https://github.com/selfcustody/krux.git |
| 71 | + |
| 72 | +# or the recommended method, via ssh: |
| 73 | +git clone git@github.com:selfcustody/krux.git |
| 74 | +``` |
| 75 | + |
| 76 | +Then follow the initial procedures described on [README.md](./README.md). |
| 77 | + |
| 78 | +### Create a topic branch |
| 79 | + |
| 80 | +You always start with a `main` branch initialized. While the |
| 81 | +community of developers standardized this as the **stable** development branch |
| 82 | +(some projects could use the `master` branch for historical reasons), |
| 83 | +the `Krux` firmware team stated the first one as the **stable** branch and the |
| 84 | +`develop` as the branch that all developers should start from and experiment with. |
| 85 | +After you have had fun and discovered things, create a properly named branch to |
| 86 | +identify when we release: |
| 87 | + |
| 88 | +> Bitcoin related projects, like bdk, pdk and floresta use |
| 89 | +`<type>/<name>` named branches. |
| 90 | + |
| 91 | + |
| 92 | +```bash |
| 93 | +main -> develop -> chore/task-stuff |
| 94 | + -> ci/job-stuff |
| 95 | + -> docs/info-new |
| 96 | + ... |
| 97 | +``` |
| 98 | + |
| 99 | +So, in command line you could do this, keeping in mind what you will do in |
| 100 | +the PR: |
| 101 | + |
| 102 | +```bash |
| 103 | +git checkout -b <type/name> |
| 104 | +``` |
| 105 | + |
| 106 | +### Named branches |
| 107 | + |
| 108 | +A patch is a set of changes (patches or `diffs`) that you propose to `Krux` |
| 109 | +firmware developers to include in `develop` branch as a (set) of commits. |
| 110 | + |
| 111 | +In general commits should be atomic and `diffs` should be easy to read. |
| 112 | +For this reason do not mix any formatting fixes or code moves with actual code |
| 113 | +changes. Further, each commit, individually, should compile and |
| 114 | +pass tests whenever possible, in order to ensure that `git bisect` and other automated tools |
| 115 | +function properly. |
| 116 | + |
| 117 | +When adding a new feature, ensure that it is covered by functional tests where |
| 118 | +possible. Avoiding this will create uncovered code. |
| 119 | + |
| 120 | +When refactoring, structure your PR to make it easy to review and don't |
| 121 | +hesitate to split it into multiple small, focused PRs. |
| 122 | + |
| 123 | +#### Minimum Supported Python Version |
| 124 | + |
| 125 | +The Minimum Supported Python Version is **3.12.0** (enforced by our CI). |
| 126 | +Commits should cover both the issue fixed and the solution's rationale. |
| 127 | + |
| 128 | +#### Conventional commits |
| 129 | + |
| 130 | +These [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in |
| 131 | +mind. Commit messages follow the |
| 132 | +["Conventional Commits 1.0.0"](https://www.conventionalcommits.org/en/v1.0.0/) |
| 133 | +to make commit histories easier to read by humans and automated tools. |
| 134 | +The types of commits we use are: |
| 135 | + |
| 136 | +- chore: maintenance tasks (mostly lint, format); |
| 137 | +- ci: continuous integration (generally `.github/**` files); |
| 138 | +- docs: documentation changes (`*.md` files); |
| 139 | +- feat: new feature (`src/**`, imperatively -- it's recommended that `test` and |
| 140 | + `docs` are accompanied in the PR); |
| 141 | +- fix: bug fix (see if it could break changes -- `!`, it's recommended that |
| 142 | + `test` and `docs` are accompanied); |
| 143 | +- refactor: code change that neither fixes a bug nor adds a feature (could be |
| 144 | + a tiny change or a entire code-base refactor -- in krux refactor is the |
| 145 | + first); |
| 146 | +- style: formatting, missing semi colons, ui colors, icons, etc; no functional |
| 147 | + code change; |
| 148 | +- test: adding missing tests or correcting existing tests. |
| 149 | + |
| 150 | +#### Signing commits |
| 151 | + |
| 152 | +It is strongly encouraged that you |
| 153 | +[GPG sign](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) |
| 154 | +your commits. |
| 155 | + |
| 156 | +--- |
| 157 | + |
| 158 | +## Peer review |
| 159 | + |
| 160 | +To make sure our code has the highest quality and is maintainable for |
| 161 | +posterity, we have a thorough peer review process, where pull requests need |
| 162 | +to be reviewed by at least one maintainer, and must not have any outstanding |
| 163 | +comment from regular contributors (unless one of maintainers recognizes the |
| 164 | +rationale). |
| 165 | + |
| 166 | +### Conceptual Review |
| 167 | + |
| 168 | +A review can be a conceptual review, where the reviewer leaves a comment: |
| 169 | + |
| 170 | +- Concept (c/)ACK: "I agree with the general goal of this pull request"; |
| 171 | +- Approach (n)ACK: "I do (not) agree with the approach of this change"; |
| 172 | +- Untested (ut)ACK: "I didn't test, but ACK"; |
| 173 | +- Tested (t)ACK: "I tested and ACKed". |
| 174 | + |
| 175 | +A nACK needs to include a rationale why the change is not worthwhile. |
| 176 | +nACKs without accompanying reasoning may be disregarded. |
| 177 | + |
| 178 | +### Code Review |
| 179 | + |
| 180 | +After conceptual agreement on the change, code review can be provided. |
| 181 | +A review begins with ACK `BRANCH_COMMIT`, where `BRANCH_COMMIT` is the top of |
| 182 | +the PR branch, followed by a description of how the reviewer did the review. |
| 183 | +The following language is used within pull request comments: |
| 184 | + |
| 185 | +- Tested ACK: |
| 186 | + |
| 187 | +> tACK `babc00fe`. I have tested the code", involving change-specific manual |
| 188 | +testing and in case it was not obvious how the manual testing was done. nit: |
| 189 | +maybe it could be described in a `docs` commit sharing your intentions. |
| 190 | + |
| 191 | +-- Untested ACK: |
| 192 | + |
| 193 | +> utACK `babc00fe`. |
| 194 | +"I have not tested the code, but I have reviewed it and it looks OK, I agree |
| 195 | +it can be merged". |
| 196 | + |
| 197 | +A "nit" refers to a trivial, often non-blocking issue. |
| 198 | + |
| 199 | +Project maintainers reserve the right to weigh the opinions of peer reviewers |
| 200 | +using common sense judgement and may also weigh based on merit. Reviewers that |
| 201 | +have demonstrated a deeper commitment and understanding of the project over |
| 202 | +time or who have clear domain expertise may naturally have more weight, as one |
| 203 | +would expect in all walks of life. |
| 204 | + |
| 205 | +```markdown |
| 206 | +tACK c00febab |
| 207 | + |
| 208 | +I liked the approach! Just some minor nit: |
| 209 | + |
| 210 | +- the line 3 has a typo on comment: `Helllo` should be `Hello`. So |
| 211 | +I suggest this change: |
| 212 | + |
| 213 | +<diff> |
| 214 | +- # The resulting Helllo |
| 215 | ++ # The resulting Hello |
| 216 | +result = "Hello" |
| 217 | +</diff> |
| 218 | + |
| 219 | +Besides that, is a neat work @user! |
| 220 | +``` |
| 221 | + |
| 222 | +--- |
| 223 | + |
| 224 | +## Coding Conventions |
| 225 | + |
| 226 | +There are a few rules to make sure the code is readable and maintainable. |
| 227 | +Most of them are checked by `poetry run poe lint` and `poetry run poe format`, |
| 228 | +and are enforced by CI, both for python and markdown codes. |
| 229 | + |
| 230 | +### Python |
| 231 | + |
| 232 | +```python |
| 233 | +# The MIT License (MIT) |
| 234 | + |
| 235 | +# Copyright (c) 2021-2026 Krux contributors |
| 236 | + |
| 237 | +# ... |
| 238 | +class Foo: |
| 239 | + """Some awesome comment""" |
| 240 | + |
| 241 | + def func(self): |
| 242 | + """Some awesome comment""" |
| 243 | + pass |
| 244 | +``` |
| 245 | + |
| 246 | +#### MIT License |
| 247 | + |
| 248 | +All files should have the MIT license with updated years from when we stated |
| 249 | +the project (2021) until today (2026 when writing this document). |
| 250 | + |
| 251 | +#### New features |
| 252 | + |
| 253 | +All new features require testing. Tests should be unique and self-describing. |
| 254 | +When it comes error handling, we prefer exact and meaningful error handling |
| 255 | +to deliver consumers (developers and users) an accurate error that describes |
| 256 | +exactly what happened wrong. Instead of: |
| 257 | + |
| 258 | +```python |
| 259 | +# A function that concatenate a int and a float into a str |
| 260 | +def foo( |
| 261 | + bar: int, |
| 262 | + baz: float, |
| 263 | +) -> str: |
| 264 | + str(bar) + str(baz) |
| 265 | +``` |
| 266 | + |
| 267 | +prefer: |
| 268 | + |
| 269 | +```python |
| 270 | +# A function that concatenate a int and a float into a str and why do that |
| 271 | +# for a good and reasonable reason that you should discuss with other developers |
| 272 | +# keep in mind that we're coding for micropython |
| 273 | +def foo(bar, baz): |
| 274 | + """Serialize an int with a float into new str |
| 275 | +
|
| 276 | + Args |
| 277 | + ---- |
| 278 | + bar(int): an int to be serialized |
| 279 | + baz(float): a float to be serialized |
| 280 | +
|
| 281 | + Returns |
| 282 | + ------- |
| 283 | + str: the serialized "<int><float>" |
| 284 | + """ |
| 285 | + bar_str = str(bar) |
| 286 | + baz_str = str(baz) |
| 287 | + |
| 288 | + # We need to concatenate to be compliant |
| 289 | + return bar_str + baz_str |
| 290 | +``` |
| 291 | + |
| 292 | +### Markdown |
| 293 | + |
| 294 | +Markdown files are linted too. This keeps the `*.md` files standardized and |
| 295 | +properly formatted for any code editor. Some of the currently applied rules enforced |
| 296 | +by CI: |
| 297 | + |
| 298 | +```markdown |
| 299 | +- \# heading should be toplevel heading; |
| 300 | +- lines should be compact: 80 chars per line; |
| 301 | +- raw links aren't allowed, use the [text](link) format instead; |
| 302 | +- items should follow correct indentation. |
| 303 | +``` |
| 304 | + |
| 305 | +--- |
| 306 | + |
| 307 | +## Testing |
| 308 | + |
| 309 | +We expect to have 95% test coverage for each PR. We have a few types of tests |
| 310 | +that you can run using `poetry run poe test`. See more with `poetry run poe`. |
| 311 | + |
| 312 | +--- |
| 313 | + |
| 314 | +## Release |
| 315 | + |
| 316 | +Once a maintainer and the contributors decide we have a stable enough `develop` |
| 317 | +with sufficient features, we will merge the `develop` branch with the `main` |
| 318 | +at that point. From this point, all new changes will go in the next release. |
| 319 | +After sufficient testing and making sure we don't have bugs left, this branch |
| 320 | +will be released by one of the maintainers. |
| 321 | + |
| 322 | +The release will have pre-built binaries available on github's asset page. |
| 323 | +They **must** be OpenSSL signed by [odudex](mailto:odudex@proton.me) -- |
| 324 | +verifiable with [`selfcustody.pem`](./selfcustody.pem), and have a `zip` |
| 325 | +accompanied with `zip.sha256.txt` release assets. |
| 326 | + |
| 327 | +If we find bugs on a release, the fix may be backported and a new minor release |
| 328 | +may be released. This is done by merging fixes on top of the release branch. |
| 329 | +And then performing another release on that branch. |
| 330 | + |
| 331 | +If you found any security issue, please read [`SECURITY.md`](./SECURITY.md), |
| 332 | +we're glad you have the choice to grow this community in a cypherpunk way. |
| 333 | + |
| 334 | +If you have any questions related to this process or the codebase in general, |
| 335 | +don't hesitate to reach out to us. We are happy to help newcomers in their amazing |
| 336 | +journey. Overall, have fun :) |
0 commit comments