development_guidelines.md
1 # Development Guidelines 2 1. [Code Documentation and Commenting](#code-documentation-and-commenting) 3 1. [Code Spacing and Formatting](#code-spacing-and-formatting) 4 1. [Additional Style Constraints](#additional-style-constraints) 5 1. [Recommended settings for your editor](#recommended-settings-for-your-editor) 6 1. [Testing](#testing) 7 1. [Model Git Commit Messages](#model-git-commit-messages) 8 1. [Ideal Git Commit Structure](#ideal-git-commit-structure) 9 1. [Sign Your Git Commits](#sign-your-git-commits) 10 1. [Pointing to Remote Dependent Branches in Go Modules](#pointing-to-remote-dependent-branches-in-go-modules) 11 1. [Use of Log Levels](#use-of-log-levels) 12 1. [Use of Golang submodules](#use-of-golang-submodules) 13 14 ## Why this emphasis on formatting? 15 16 Code in general (and Open Source code specifically) is _read_ by developers many 17 more times during its lifecycle than it is modified. With this fact in mind, the 18 Golang language was designed for readability (among other goals). 19 While the enforced formatting of `go fmt` and some best practices already 20 eliminate many discussions, the resulting code can still look and feel very 21 differently among different developers. 22 23 We aim to enforce a few additional rules to unify the look and feel of all code 24 in `lnd` to help improve the overall readability. 25 26 ## Code Documentation and Commenting 27 28 - At a minimum every function must be commented with its intended purpose and 29 any assumptions that it makes 30 - Function comments must always begin with the name of the function per 31 [Effective Go](https://golang.org/doc/effective_go.html) 32 - Function comments should be complete sentences since they allow a wide 33 variety of automated presentations such as [godoc.org](https://godoc.org) 34 - The general rule of thumb is to look at it as if you were completely 35 unfamiliar with the code and ask yourself, would this give me enough 36 information to understand what this function does and how I'd probably want to 37 use it? 38 - Exported functions should also include detailed information the caller of the 39 function will likely need to know and/or understand:<br /><br /> 40 41 **WRONG** 42 ```go 43 // generates a revocation key 44 func DeriveRevocationPubkey(commitPubKey *btcec.PublicKey, 45 revokePreimage []byte) *btcec.PublicKey { 46 ``` 47 **RIGHT** 48 ```go 49 // DeriveRevocationPubkey derives the revocation public key given the 50 // counterparty's commitment key, and revocation preimage derived via a 51 // pseudo-random-function. In the event that we (for some reason) broadcast a 52 // revoked commitment transaction, then if the other party knows the revocation 53 // preimage, then they'll be able to derive the corresponding private key to 54 // this private key by exploiting the homomorphism in the elliptic curve group: 55 // * https://en.wikipedia.org/wiki/Group_homomorphism#Homomorphisms_of_abelian_groups 56 // 57 // The derivation is performed as follows: 58 // 59 // revokeKey := commitKey + revokePoint 60 // := G*k + G*h 61 // := G * (k+h) 62 // 63 // Therefore, once we divulge the revocation preimage, the remote peer is able to 64 // compute the proper private key for the revokeKey by computing: 65 // revokePriv := commitPriv + revokePreimge mod N 66 // 67 // Where N is the order of the sub-group. 68 func DeriveRevocationPubkey(commitPubKey *btcec.PublicKey, 69 revokePreimage []byte) *btcec.PublicKey { 70 ``` 71 - Comments in the body of the code are highly encouraged, but they should 72 explain the intention of the code as opposed to just calling out the 73 obvious<br /><br /> 74 75 **WRONG** 76 ```go 77 // return err if amt is less than 546 78 if amt < 546 { 79 return err 80 } 81 ``` 82 **RIGHT** 83 ```go 84 // Treat transactions with amounts less than the amount which is considered dust 85 // as non-standard. 86 if amt < 546 { 87 return err 88 } 89 ``` 90 **NOTE:** The above should really use a constant as opposed to a magic number, 91 but it was left as a magic number to show how much of a difference a good 92 comment can make. 93 94 ## Code Spacing and formatting 95 96 Code in general (and Open Source code specifically) is _read_ by developers many 97 more times during its lifecycle than it is modified. With this fact in mind, the 98 Golang language was designed for readability (among other goals). 99 While the enforced formatting of `go fmt` and some best practices already 100 eliminate many discussions, the resulting code can still look and feel very 101 differently among different developers. 102 103 We aim to enforce a few additional rules to unify the look and feel of all code 104 in `lnd` to help improve the overall readability. 105 106 Blocks of code within `lnd` should be segmented into logical stanzas of 107 operation. Such spacing makes the code easier to follow at a skim, and reduces 108 unnecessary line noise. Coupled with the commenting scheme specified in the 109 [contribution guide](#code-documentation-and-commenting), 110 proper spacing allows readers to quickly scan code, extracting semantics quickly. 111 Functions should _not_ just be laid out as a bare contiguous block of code. 112 113 **WRONG** 114 ```go 115 witness := make([][]byte, 4) 116 witness[0] = nil 117 if bytes.Compare(pubA, pubB) == -1 { 118 witness[1] = sigB 119 witness[2] = sigA 120 } else { 121 witness[1] = sigA 122 witness[2] = sigB 123 } 124 witness[3] = witnessScript 125 return witness 126 ``` 127 **RIGHT** 128 ```go 129 witness := make([][]byte, 4) 130 131 // When spending a p2wsh multi-sig script, rather than an OP_0, we add 132 // a nil stack element to eat the extra pop. 133 witness[0] = nil 134 135 // When initially generating the witnessScript, we sorted the serialized 136 // public keys in descending order. So we do a quick comparison in order 137 // to ensure the signatures appear on the Script Virtual Machine stack in 138 // the correct order. 139 if bytes.Compare(pubA, pubB) == -1 { 140 witness[1] = sigB 141 witness[2] = sigA 142 } else { 143 witness[1] = sigA 144 witness[2] = sigB 145 } 146 147 // Finally, add the preimage as the last witness element. 148 witness[3] = witnessScript 149 150 return witness 151 ``` 152 153 Additionally, we favor spacing between stanzas within syntax like: switch case 154 statements and select statements. 155 156 **WRONG** 157 ```go 158 switch { 159 case a: 160 <code block> 161 case b: 162 <code block> 163 case c: 164 <code block> 165 case d: 166 <code block> 167 default: 168 <code block> 169 } 170 ``` 171 **RIGHT** 172 ```go 173 switch { 174 // Brief comment detailing instances of this case (repeat below). 175 case a: 176 <code block> 177 178 case b: 179 <code block> 180 181 case c: 182 <code block> 183 184 case d: 185 <code block> 186 187 default: 188 <code block> 189 } 190 ``` 191 192 ## Additional Style Constraints 193 194 Before a PR is submitted, the proposer should ensure that the file passes the 195 set of linting scripts run by `make lint`. These include `gofmt`. In addition 196 to `gofmt` we've opted to enforce the following style guidelines. 197 198 ### 80 character line length 199 200 ALL columns (on a best effort basis) should be wrapped to 80 line columns. 201 Editors should be set to treat a **tab as 8 spaces**. 202 203 **WRONG** 204 ```go 205 myKey := "0214cd678a565041d00e6cf8d62ef8add33b4af4786fb2beb87b366a2e151fcee7" 206 ``` 207 208 **RIGHT** 209 ```go 210 myKey := "0214cd678a565041d00e6cf8d62ef8add33b4af4786fb2beb87b366a2e1" + 211 "51fcee7" 212 ``` 213 214 ### Wrapping long function calls 215 216 When wrapping a line that contains a function call as the unwrapped line exceeds 217 the column limit, the close parenthesis should be placed on its own line. 218 Additionally, all arguments should begin in a new line after the open parenthesis. 219 220 **WRONG** 221 ```go 222 value, err := bar(a, 223 a, b, c) 224 ``` 225 226 **RIGHT** 227 ```go 228 value, err := bar( 229 a, a, b, c, 230 ) 231 ``` 232 233 As long as the visual symmetry of the opening and closing parentheses (or curly 234 braces) is preserved, arguments that would otherwise introduce a new level of 235 indentation are allowed to be written in a more compact form. 236 Visual symmetry here means that when two or more opening parentheses or curly 237 braces are on the same line, then they must also be closed on the same line. 238 And the closing line needs to have the same indentation level as the opening 239 line. 240 241 Example with inline struct creation: 242 243 **ACCEPTABLE** 244 ```go 245 response, err := node.AddInvoice( 246 ctx, &lnrpc.Invoice{ 247 Memo: "invoice", 248 ValueMsat: int64(oneUnitMilliSat - 1), 249 }, 250 ) 251 ``` 252 253 **PREFERRED** 254 ```go 255 response, err := node.AddInvoice(ctx, &lnrpc.Invoice{ 256 Memo: "invoice", 257 ValueMsat: int64(oneUnitMilliSat - 1), 258 }) 259 ``` 260 261 **WRONG** 262 ```go 263 response, err := node.AddInvoice(ctx, &lnrpc.Invoice{ 264 Memo: "invoice", 265 ValueMsat: int64(oneUnitMilliSat - 1)}) 266 ``` 267 268 Example with nested function call: 269 270 **ACCEPTABLE**: 271 ```go 272 payInvoiceWithSatoshi( 273 t.t, dave, invoiceResp2, withFailure( 274 lnrpc.Payment_FAILED, failureNoRoute, 275 ), 276 ) 277 ``` 278 279 **PREFERRED**: 280 ```go 281 payInvoiceWithSatoshi(t.t, dave, invoiceResp2, withFailure( 282 lnrpc.Payment_FAILED, failureNoRoute, 283 )) 284 ``` 285 286 #### Exception for log and error message formatting 287 288 **Note that the above guidelines don't apply to log or error messages.** For 289 log and error messages, committers should attempt to minimize the number of 290 lines utilized, while still adhering to the 80-character column limit. For 291 example: 292 293 **WRONG** 294 ```go 295 return fmt.Errorf( 296 "this is a long error message with a couple (%d) place holders", 297 len(things), 298 ) 299 300 log.Debugf( 301 "Something happened here that we need to log: %v", 302 longVariableNameHere, 303 ) 304 ``` 305 306 **RIGHT** 307 ```go 308 return fmt.Errorf("this is a long error message with a couple (%d) place "+ 309 "holders", len(things)) 310 311 log.Debugf("Something happened here that we need to log: %v", 312 longVariableNameHere) 313 ``` 314 315 This helps to visually distinguish those formatting statements (where nothing 316 of consequence happens except for formatting an error message or writing 317 to a log) from actual method or function calls. This compact formatting should 318 be used for calls to formatting functions like `fmt.Errorf`, 319 `log.(Trace|Debug|Info|Warn|Error)f` and `fmt.Printf`. 320 But not for statements that are important for the flow or logic of the code, 321 like `require.NoErrorf()`. 322 323 #### Exceptions and additional styling for structured logging 324 325 When making use of structured logging calls (there are any `btclog.Logger` 326 methods ending in `S`), a few different rules and exceptions apply. 327 328 1) **Static messages:** Structured log calls take a `context.Context` as a first 329 parameter and a _static_ string as the second parameter (the `msg` parameter). 330 Formatted strings should ideally not be used for the construction of the `msg` 331 parameter. Instead, key-value pairs (or `slog` attributes) should be used to 332 provide additional variables to the log line. 333 334 **WRONG** 335 ```go 336 log.DebugS(ctx, fmt.Sprintf("User %d just spent %.8f to open a channel", userID, 0.0154)) 337 ``` 338 339 **RIGHT** 340 ```go 341 log.InfoS(ctx, "Channel open performed", 342 slog.Int("user_id", userID), 343 btclog.Fmt("amount", "%.8f", 0.00154)) 344 ``` 345 346 2) **Key-value attributes**: The third parameter in any structured log method is 347 a variadic list of the `any` type but it is required that these are provided in 348 key-value pairs such that an associated `slog.Attr` variable can be created for 349 each key-value pair. The simplest way to specify this is to directly pass in the 350 key-value pairs as raw literals as follows: 351 352 ```go 353 log.InfoS(ctx, "Channel open performed", "user_id", userID, "amount", 0.00154) 354 ``` 355 This does work, but it becomes easy to make a mistake and accidentally leave out 356 a value for each key provided leading to a nonsensical log line. To avoid this, 357 it is suggested to make use of the various `slog.Attr` helper functions as 358 follows: 359 360 ```go 361 log.InfoS(ctx, "Channel open performed", 362 slog.Int("user_id", userID), 363 btclog.Fmt("amount", "%.8f", 0.00154)) 364 ``` 365 366 3) **Line wrapping**: Structured log lines are an exception to the 80-character 367 line wrapping rule. This is so that the key-value pairs can be easily read and 368 reasoned about. If it is the case that there is only a single key-value pair 369 and the entire log line is still less than 80 characters, it is acceptable to 370 have the key-value pair on the same line as the log message. However, if there 371 are multiple key-value pairs, it is suggested to use the one line per key-value 372 pair format. Due to this suggestion, it is acceptable for any single key-value 373 pair line to exceed 80 characters for the sake of readability. 374 375 **WRONG** 376 ```go 377 // Example 1. 378 log.InfoS(ctx, "User connected", 379 "user_id", userID) 380 381 // Example 2. 382 log.InfoS(ctx, "Channel open performed", "user_id", userID, 383 btclog.Fmt("amount", "%.8f", 0.00154), "channel_id", channelID) 384 385 // Example 3. 386 log.InfoS(ctx, "Bytes received", 387 "user_id", userID, 388 btclog.Hex("peer_id", peerID.SerializeCompressed()), 389 btclog.Hex("message", []bytes{ 390 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 391 }))) 392 ``` 393 394 **RIGHT** 395 ```go 396 // Example 1. 397 log.InfoS(ctx, "User connected", "user_id", userID) 398 399 // Example 2. 400 log.InfoS(ctx, "Channel open performed", 401 slog.Int("user_id", userID), 402 btclog.Fmt("amount", "%.8f", 0.00154), 403 slog.String("channel_id", channelID)) 404 405 // Example 3. 406 log.InfoS(ctx, "Bytes received", 407 "user_id", userID, 408 btclog.Hex("peer_id", peerID.SerializeCompressed()), 409 btclog.Hex("message", []bytes{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}))) 410 ``` 411 412 ### Wrapping long function definitions 413 414 If one is forced to wrap lines of function arguments that exceed the 415 80-character limit, then indentation must be kept on the following lines. Also, 416 lines should not end with an open parenthesis if the function definition isn't 417 finished yet. 418 419 **WRONG** 420 ```go 421 func foo(a, b, c, 422 ) (d, error) { 423 424 func bar(a, b, c) ( 425 d, error, 426 ) { 427 428 func baz(a, b, c) ( 429 d, error) { 430 ``` 431 **RIGHT** 432 ```go 433 func foo(a, b, 434 c) (d, error) { 435 436 func baz(a, b, c) (d, 437 error) { 438 439 func longFunctionName( 440 a, b, c) (d, error) { 441 ``` 442 443 If a function declaration spans multiple lines the body should start with an 444 empty line to help visually distinguishing the two elements. 445 446 **WRONG** 447 ```go 448 func foo(a, b, c, 449 d, e) error { 450 var a int 451 } 452 ``` 453 **RIGHT** 454 ```go 455 func foo(a, b, c, 456 d, e) error { 457 458 var a int 459 } 460 ``` 461 462 ### Inline slice definitions 463 464 In Go a list of slices can be initialized with values directly, using curly 465 braces. Whenever possible, the more verbose/indented style should be used for 466 better readability and easier git diff handling. Because that results in more 467 levels of code indentation, the more compact version is allowed in situations 468 where the remaining space would otherwise be too restricted, resulting in too 469 long lines (or excessive use of the `// nolint: ll` directive). 470 471 **ACCEPTABLE** 472 ```go 473 testCases := []testCase{{ 474 name: "spend exactly all", 475 coins: []wallet.Coin{{ 476 TxOut: wire.TxOut{ 477 PkScript: p2wkhScript, 478 Value: 1 * btcutil.SatoshiPerBitcoin, 479 }, 480 }}, 481 }, { 482 name: "spend more", 483 coins: []wallet.Coin{{ 484 TxOut: wire.TxOut{ 485 PkScript: p2wkhScript, 486 Value: 1 * btcutil.SatoshiPerBitcoin, 487 }, 488 }}, 489 }} 490 ``` 491 492 **PREFERRED** 493 ```go 494 coin := btcutil.SatoshiPerBitcoin 495 testCases := []testCase{ 496 { 497 name: "spend exactly all", 498 coins: []wallet.Coin{ 499 { 500 TxOut: wire.TxOut{ 501 PkScript: p2wkhScript, 502 Value: 1 * coin, 503 }, 504 }, 505 }, 506 }, 507 { 508 name: "spend more", 509 coins: []wallet.Coin{ 510 { 511 TxOut: wire.TxOut{ 512 PkScript: p2wkhScript, 513 Value: 1 * coin, 514 }, 515 }, 516 }, 517 }, 518 } 519 ``` 520 521 ## Recommended settings for your editor 522 523 To make it easier to follow the rules outlined above, we recommend setting up 524 your editor with at least the following two settings: 525 526 1. Set your tabulator width (also called "tab size") to **8 spaces**. 527 2. Set a ruler or visual guide at 80 character. 528 529 Note that the two above settings are automatically applied in editors that 530 support the `EditorConfig` scheme (for example GoLand, GitHub, GitLab, 531 VisualStudio). In addition, specific settings for Visual Studio Code are checked 532 into the code base as well. 533 534 Other editors (for example Atom, Notepad++, Vim, Emacs and so on) might install 535 a plugin to understand the rules in the `.editorconfig` file. 536 537 In Vim, you might want to use `set colorcolumn=80`. 538 539 540 ## Testing 541 542 One of the major design goals of all of `lnd`'s packages and the daemon itself is 543 to aim for a high degree of test coverage. This is financial software so bugs 544 and regressions in the core logic can cost people real money. For this reason 545 every effort must be taken to ensure the code is as accurate and bug-free as 546 possible. Thorough testing is a good way to help achieve that goal. 547 548 Unless a new feature you submit is completely trivial, it will probably be 549 rejected unless it is also accompanied by adequate test coverage for both 550 positive and negative conditions. That is to say, the tests must ensure your 551 code works correctly when it is fed correct data as well as incorrect data 552 (error paths). 553 554 555 Go provides an excellent test framework that makes writing test code and 556 checking coverage statistics straightforward. For more information about the 557 test coverage tools, see the [golang cover blog post](https://blog.golang.org/cover). 558 559 A quick summary of test practices follows: 560 561 - All new code should be accompanied by tests that ensure the code behaves 562 correctly when given expected values, and, perhaps even more importantly, that 563 it handles errors gracefully. 564 - When you fix a bug, it should be accompanied by tests which exercise the bug 565 to both prove it has been resolved and to prevent future regressions. 566 - Changes to publicly exported packages such as 567 [brontide](https://github.com/lightningnetwork/lnd/tree/master/brontide) should 568 be accompanied by unit tests exercising the new or changed behavior. 569 - Changes to behavior within the daemon's interaction with the P2P protocol, 570 or RPC's will need to be accompanied by integration tests which use the 571 [`networkHarness`framework](https://github.com/lightningnetwork/lnd/blob/master/lntest/harness.go) 572 contained within `lnd`. For example integration tests, see 573 [`lnd_test.go`](https://github.com/lightningnetwork/lnd/blob/master/itest/lnd_test.go). 574 - The itest log files are automatically scanned for `[ERR]` lines. There 575 shouldn't be any of those in the logs, see [Use of Log Levels](#use-of-log-levels). 576 577 Throughout the process of contributing to `lnd`, you'll likely also be 578 extensively using the commands within our `Makefile`. As a result, we recommend 579 [perusing the make file documentation](https://github.com/lightningnetwork/lnd/blob/master/docs/MAKEFILE.md). 580 581 582 Before committing the changes made, you should run unit tests to validate the 583 changes, and provide a new integration test when necessary. The unit tests 584 should pass at two levels, 585 586 - Run `make unit-debug log="stdlog trace" pkg=$pkg case=$case timeout=10s`, 587 where `pkg` is the package that's updated, and `case` is the newly added or 588 affected unit test. This command should run against all the newly added and 589 affected test cases. In addition, you should pay attention to the logs added 590 here, and make sure they are correctly formatted and no spammy. Also notice 591 the timeout - 10 seconds should be more than enough to run a single unit test 592 case, if it takes longer than that, consider break the test to make it more 593 "unit". 594 595 - Run `make unit pkg=$pkg timeout=5m` to make sure all existing unit tests still 596 pass. 597 598 - If there are newly added integration tests, or the changes may alter the 599 workflow of specific areas, run `make itest icase=$icase` to validate the 600 behavior, where `icase` is the affected test case. 601 602 603 ## Model Git Commit Messages 604 605 This project prefers to keep a clean commit history with well-formed commit 606 messages. This section illustrates a model commit message and provides a bit 607 of background for it. This content was originally created by Tim Pope and made 608 available on his website, however that website is no longer active, so it is 609 being provided here. 610 611 Here’s a model Git commit message: 612 613 ```text 614 Short (50 chars or less) summary of changes 615 616 More detailed explanatory text, if necessary. Wrap it to about 72 617 characters or so. In some contexts, the first line is treated as the 618 subject of an email and the rest of the text as the body. The blank 619 line separating the summary from the body is critical (unless you omit 620 the body entirely); tools like rebase can get confused if you run the 621 two together. 622 623 Write your commit message in the present tense: "Fix bug" and not "Fixed 624 bug." This convention matches up with commit messages generated by 625 commands like git merge and git revert. 626 627 Further paragraphs come after blank lines. 628 629 - Bullet points are okay, too 630 - Typically a hyphen or asterisk is used for the bullet, preceded by a 631 single space, with blank lines in between, but conventions vary here 632 - Use a hanging indent 633 ``` 634 635 Here are some of the reasons why wrapping your commit messages to 72 columns is 636 a good thing. 637 638 - git log doesn't do any special wrapping of the commit messages. With 639 the default pager of less -S, this means your paragraphs flow far off the edge 640 of the screen, making them difficult to read. On an 80 column terminal, if we 641 subtract 4 columns for the indent on the left and 4 more for symmetry on the 642 right, we’re left with 72 columns. 643 - git format-patch --stdout converts a series of commits to a series of emails, 644 using the messages for the message body. Good email netiquette dictates we 645 wrap our plain text emails such that there’s room for a few levels of nested 646 reply indicators without overflow in an 80 column terminal. 647 648 649 In addition to the Git commit message structure adhered to within the daemon 650 all short-[commit messages are to be prefixed according to the convention 651 outlined in the Go project](https://golang.org/doc/contribute.html#change). All 652 commits should begin with the subsystem or package primarily affected by the 653 change. In the case of a widespread change, the packages are to be delimited by 654 either a '+' or a ','. This prefix seems minor but can be extremely helpful in 655 determining the scope of a commit at a glance, or when bug hunting to find a 656 commit which introduced a bug or regression. 657 658 ## Ideal Git Commit Structure 659 660 Within the project we prefer small, contained commits for a pull request over a 661 single giant commit that touches several files/packages. Ideal commits build on 662 their own, in order to facilitate easy usage of tools like `git bisect` to `git 663 cherry-pick`. It's preferred that commits contain an isolated change in a 664 single package. In this case, the commit header message should begin with the 665 prefix of the modified package. For example, if a commit was made to modify the 666 `lnwallet` package, it should start with `lnwallet: `. 667 668 In the case of changes that only build in tandem with changes made in other 669 packages, it is permitted for a single commit to be made which contains several 670 prefixes such as: `lnwallet+htlcswitch`. This prefix structure along with the 671 requirement for atomic contained commits (when possible) make things like 672 scanning the set of commits and debugging easier. In the case of changes that 673 touch several packages, and can only compile with the change across several 674 packages, a `multi: ` prefix should be used. 675 676 Examples of common patterns w.r.t commit structures within the project: 677 678 * It is common that during the work on a PR, existing bugs are found and 679 fixed. If they can be fixed in isolation, they should have their own 680 commit. 681 * File restructuring like moving a function to another file or changing order 682 of functions: with a separate commit because it is much easier to review 683 the real changes that go on top of the restructuring. 684 * Preparatory refactorings that are functionally equivalent: own commit. 685 * Project or package wide file renamings should be in their own commit. 686 * Ideally if a new package/struct/sub-system is added in a PR, there should 687 be a single commit which adds the new functionality, with follow up 688 individual commits that begin to integrate the functionality within the 689 codebase. 690 * If a PR only fixes a trivial issue, such as updating documentation on a 691 small scale, fix typos, or any changes that do not modify the code, the 692 commit message of the HEAD commit of the PR should end with `[skip ci]` to 693 skip the CI checks. When pushing to such an existing PR, the latest commit 694 being pushed should end with `[skip ci]` as to not inadvertently trigger the 695 CI checks. 696 697 ## Sign your git commits 698 699 When contributing to `lnd` it is recommended to sign your git commits. This is 700 easy to do and will help in assuring the integrity of the tree. See [mailing 701 list entry](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2014-May/005877.html) 702 for more information. 703 704 ### How to sign your commits? 705 706 Provide the `-S` flag (or `--gpg-sign`) to git commit when you commit 707 your changes, for example 708 709 ```shell 710 $ git commit -m "Commit message" -S 711 ``` 712 713 Optionally you can provide a key id after the `-S` option to sign with a 714 specific key. 715 716 To instruct `git` to auto-sign every commit, add the following lines to your 717 `~/.gitconfig` file: 718 719 ```text 720 [commit] 721 gpgsign = true 722 ``` 723 724 ### What if I forgot? 725 726 You can retroactively sign your previous commit using `--amend`, for example 727 728 ```shell 729 $ git commit -S --amend 730 ``` 731 732 If you need to go further back, you can use the interactive rebase 733 command with 'edit'. Replace `HEAD~3` with the base commit from which 734 you want to start. 735 736 ```shell 737 $ git rebase -i HEAD~3 738 ``` 739 740 Replace 'pick' by 'edit' for the commit that you want to sign and the 741 rebasing will stop after that commit. Then you can amend the commit as 742 above. Afterwards, do 743 744 ```shell 745 $ git rebase --continue 746 ``` 747 748 As this will rewrite history, you cannot do this when your commit is 749 already merged. In that case, too bad, better luck next time. 750 751 If you rewrite history for another reason - for example when squashing 752 commits - make sure that you re-sign as the signatures will be lost. 753 754 Multiple commits can also be re-signed with `git rebase`. For example, signing 755 the last three commits can be done with: 756 757 ```shell 758 $ git rebase --exec 'git commit --amend --no-edit -n -S' -i HEAD~3 759 ``` 760 761 ### How to check if commits are signed? 762 763 Use `git log` with `--show-signature`, 764 765 ```shell 766 $ git log --show-signature 767 ``` 768 769 You can also pass the `--show-signature` option to `git show` to check a single 770 commit. 771 772 773 ## Pointing to Remote Dependent Branches in Go Modules 774 775 It's common that a developer may need to make a change in a dependent project 776 of `lnd` such as `btcd`, `neutrino`, `btcwallet`, etc. In order to test changes 777 without testing infrastructure, or simply make a PR into `lnd` that will build 778 without any further work, the `go.mod` and `go.sum` files will need to be 779 updated. Luckily, the `go mod` command has a handy tool to do this 780 automatically so developers don't need to manually edit the `go.mod` file: 781 ```shell 782 $ go mod edit -replace=IMPORT-PATH-IN-LND@LND-VERSION=DEV-FORK-IMPORT-PATH@DEV-FORK-VERSION 783 ``` 784 785 Here's an example replacing the `lightning-onion` version checked into `lnd` with a version in roasbeef's fork: 786 ```shell 787 $ go mod edit -replace=github.com/lightningnetwork/lightning-onion@v0.0.0-20180605012408-ac4d9da8f1d6=github.com/roasbeef/lightning-onion@2e5ae87696046298365ab43bcd1cf3a7a1d69695 788 ``` 789 790 ## Use of Log Levels 791 792 There are six log levels available: `trace`, `debug`, `info`, `warn`, `error` and `critical`. 793 794 Only use `error` for internal errors that are never expected to happen during 795 normal operation. No event triggered by external sources (rpc, chain backend, 796 etc) should lead to an `error` log. 797 798 ## Use of Golang submodules 799 800 Changes to packages that are their own submodules (e.g. they contain a `go.mod` 801 and `go.sum` file, for example `tor/go.mod`) require a specific process. 802 We want to avoid the use of local replace directives in the root `go.mod`, 803 therefore changes to a submodule are a bit involved. 804 805 The main process for updating and then using code in a submodule is as follows: 806 - Create a PR for the changes to the submodule itself (e.g. edit something in 807 the `tor` package) 808 - Wait for the PR to be merged and a new tag (for example `tor/v1.0.x`) to be 809 pushed. 810 - Create a second PR that bumps the updated submodule in the root `go.mod` and 811 uses the new functionality in the main module. 812 813 Of course the two PRs can be opened at the same time and be built on top of each 814 other. But the merge and tag push order should always be maintained.