|
| 1 | +# LND Style Guide |
| 2 | + |
| 3 | +## Code Documentation and Commenting |
| 4 | + |
| 5 | +- Always use the Golang code style described below in this document. |
| 6 | +- Readable code is the most important requirement for any commit created. |
| 7 | +- Comments must not explain the code 1:1 but instead explain the _why_ behind a |
| 8 | + certain block of code, in case it requires contextual knowledge. |
| 9 | +- Unit tests must always use the `require` library. Either table driven unit |
| 10 | + tests or tests using the `rapid` library are preferred. |
| 11 | +- The line length MUST NOT exceed 80 characters, this is very important. |
| 12 | + You must count the Golang indentation (tabulator character) as 8 spaces when |
| 13 | + determining the line length. Use creative approaches or the wrapping rules |
| 14 | + specified below to make sure the line length isn't exceeded. |
| 15 | +- Every function must be commented with its purpose and assumptions. |
| 16 | +- Function comments must begin with the function name. |
| 17 | +- Function comments should be complete sentences. |
| 18 | +- Exported functions require detailed comments for the caller. |
| 19 | + |
| 20 | +**WRONG** |
| 21 | +```go |
| 22 | +// generates a revocation key |
| 23 | +func DeriveRevocationPubkey(commitPubKey *btcec.PublicKey, |
| 24 | + revokePreimage []byte) *btcec.PublicKey { |
| 25 | +``` |
| 26 | +**RIGHT** |
| 27 | +```go |
| 28 | +// DeriveRevocationPubkey derives the revocation public key given the |
| 29 | +// counterparty's commitment key, and revocation preimage derived via a |
| 30 | +// pseudo-random-function. In the event that we (for some reason) broadcast a |
| 31 | +// revoked commitment transaction, then if the other party knows the revocation |
| 32 | +// preimage, then they'll be able to derive the corresponding private key to |
| 33 | +// this private key by exploiting the homomorphism in the elliptic curve group. |
| 34 | +// |
| 35 | +// The derivation is performed as follows: |
| 36 | +// |
| 37 | +// revokeKey := commitKey + revokePoint |
| 38 | +// := G*k + G*h |
| 39 | +// := G * (k+h) |
| 40 | +// |
| 41 | +// Therefore, once we divulge the revocation preimage, the remote peer is able |
| 42 | +// to compute the proper private key for the revokeKey by computing: |
| 43 | +// revokePriv := commitPriv + revokePreimge mod N |
| 44 | +// |
| 45 | +// Where N is the order of the sub-group. |
| 46 | +func DeriveRevocationPubkey(commitPubKey *btcec.PublicKey, |
| 47 | + revokePreimage []byte) *btcec.PublicKey { |
| 48 | +``` |
| 49 | +- In-body comments should explain the *intention* of the code. |
| 50 | +
|
| 51 | +**WRONG** |
| 52 | +```go |
| 53 | +// return err if amt is less than 546 |
| 54 | +if amt < 546 { |
| 55 | + return err |
| 56 | +} |
| 57 | +``` |
| 58 | +**RIGHT** |
| 59 | +```go |
| 60 | +// Treat transactions with amounts less than the amount which is considered dust |
| 61 | +// as non-standard. |
| 62 | +if amt < 546 { |
| 63 | + return err |
| 64 | +} |
| 65 | +``` |
| 66 | +
|
| 67 | +## Code Spacing and formatting |
| 68 | +
|
| 69 | +- Segment code into logical stanzas separated by newlines. |
| 70 | +
|
| 71 | +**WRONG** |
| 72 | +```go |
| 73 | + witness := make([][]byte, 4) |
| 74 | + witness[0] = nil |
| 75 | + if bytes.Compare(pubA, pubB) == -1 { |
| 76 | + witness[1] = sigB |
| 77 | + witness[2] = sigA |
| 78 | + } else { |
| 79 | + witness[1] = sigA |
| 80 | + witness[2] = sigB |
| 81 | + } |
| 82 | + witness[3] = witnessScript |
| 83 | + return witness |
| 84 | +``` |
| 85 | +**RIGHT** |
| 86 | +```go |
| 87 | + witness := make([][]byte, 4) |
| 88 | + |
| 89 | + // When spending a p2wsh multi-sig script, rather than an OP_0, we add |
| 90 | + // a nil stack element to eat the extra pop. |
| 91 | + witness[0] = nil |
| 92 | + |
| 93 | + // When initially generating the witnessScript, we sorted the serialized |
| 94 | + // public keys in descending order. So we do a quick comparison in order |
| 95 | + // to ensure the signatures appear on the Script Virtual Machine stack in |
| 96 | + // the correct order. |
| 97 | + if bytes.Compare(pubA, pubB) == -1 { |
| 98 | + witness[1] = sigB |
| 99 | + witness[2] = sigA |
| 100 | + } else { |
| 101 | + witness[1] = sigA |
| 102 | + witness[2] = sigB |
| 103 | + } |
| 104 | + |
| 105 | + // Finally, add the preimage as the last witness element. |
| 106 | + witness[3] = witnessScript |
| 107 | + |
| 108 | + return witness |
| 109 | +``` |
| 110 | +
|
| 111 | +- Use spacing between `case` and `select` stanzas. |
| 112 | +
|
| 113 | +**WRONG** |
| 114 | +```go |
| 115 | + switch { |
| 116 | + case a: |
| 117 | + <code block> |
| 118 | + case b: |
| 119 | + <code block> |
| 120 | + case c: |
| 121 | + <code block> |
| 122 | + case d: |
| 123 | + <code block> |
| 124 | + default: |
| 125 | + <code block> |
| 126 | + } |
| 127 | +``` |
| 128 | +**RIGHT** |
| 129 | +```go |
| 130 | + switch { |
| 131 | + // Brief comment detailing instances of this case (repeat below). |
| 132 | + case a: |
| 133 | + <code block> |
| 134 | + |
| 135 | + case b: |
| 136 | + <code block> |
| 137 | + |
| 138 | + case c: |
| 139 | + <code block> |
| 140 | + |
| 141 | + case d: |
| 142 | + <code block> |
| 143 | + |
| 144 | + default: |
| 145 | + <code block> |
| 146 | + } |
| 147 | +``` |
| 148 | +
|
| 149 | +## Additional Style Constraints |
| 150 | +
|
| 151 | +### 80 character line length |
| 152 | +
|
| 153 | +- Wrap columns at 80 characters. |
| 154 | +- Tabs are 8 spaces. |
| 155 | +
|
| 156 | +**WRONG** |
| 157 | +```go |
| 158 | +myKey := "0214cd678a565041d00e6cf8d62ef8add33b4af4786fb2beb87b366a2e151fcee7" |
| 159 | +``` |
| 160 | +
|
| 161 | +**RIGHT** |
| 162 | +```go |
| 163 | +myKey := "0214cd678a565041d00e6cf8d62ef8add33b4af4786fb2beb87b366a2e1" + |
| 164 | + "51fcee7" |
| 165 | +``` |
| 166 | +
|
| 167 | +### Wrapping long function calls |
| 168 | +
|
| 169 | +- If a function call exceeds the column limit, place the closing parenthesis |
| 170 | + on its own line and start all arguments on a new line after the opening |
| 171 | + parenthesis. |
| 172 | +
|
| 173 | +**WRONG** |
| 174 | +```go |
| 175 | +value, err := bar(a, |
| 176 | + a, b, c) |
| 177 | +``` |
| 178 | +
|
| 179 | +**RIGHT** |
| 180 | +```go |
| 181 | +value, err := bar( |
| 182 | + a, a, b, c, |
| 183 | +) |
| 184 | +``` |
| 185 | +
|
| 186 | +- Compact form is acceptable if visual symmetry of parentheses is preserved. |
| 187 | +
|
| 188 | +**ACCEPTABLE** |
| 189 | +```go |
| 190 | + response, err := node.AddInvoice( |
| 191 | + ctx, &lnrpc.Invoice{ |
| 192 | + Memo: "invoice", |
| 193 | + ValueMsat: int64(oneUnitMilliSat - 1), |
| 194 | + }, |
| 195 | + ) |
| 196 | +``` |
| 197 | +
|
| 198 | +**PREFERRED** |
| 199 | +```go |
| 200 | + response, err := node.AddInvoice(ctx, &lnrpc.Invoice{ |
| 201 | + Memo: "invoice", |
| 202 | + ValueMsat: int64(oneUnitMilliSat - 1), |
| 203 | + }) |
| 204 | +``` |
| 205 | +
|
| 206 | +### Exception for log and error message formatting |
| 207 | +
|
| 208 | +- Minimize lines for log and error messages, while adhering to the |
| 209 | + 80-character limit. |
| 210 | +
|
| 211 | +**WRONG** |
| 212 | +```go |
| 213 | +return fmt.Errorf( |
| 214 | + "this is a long error message with a couple (%d) place holders", |
| 215 | + len(things), |
| 216 | +) |
| 217 | + |
| 218 | +log.Debugf( |
| 219 | + "Something happened here that we need to log: %v", |
| 220 | + longVariableNameHere, |
| 221 | +) |
| 222 | +``` |
| 223 | +
|
| 224 | +**RIGHT** |
| 225 | +```go |
| 226 | +return fmt.Errorf("this is a long error message with a couple (%d) place "+ |
| 227 | + "holders", len(things)) |
| 228 | + |
| 229 | +log.Debugf("Something happened here that we need to log: %v", |
| 230 | + longVariableNameHere) |
| 231 | +``` |
| 232 | +
|
| 233 | +### Exceptions and additional styling for structured logging |
| 234 | +
|
| 235 | +- **Static messages:** Use key-value pairs instead of formatted strings for the |
| 236 | + `msg` parameter. |
| 237 | +- **Key-value attributes:** Use `slog.Attr` helper functions. |
| 238 | +- **Line wrapping:** Structured log lines are an exception to the 80-character |
| 239 | + rule. Use one line per key-value pair for multiple attributes. |
| 240 | +
|
| 241 | +**WRONG** |
| 242 | +```go |
| 243 | +log.DebugS(ctx, fmt.Sprintf("User %d just spent %.8f to open a channel", userID, 0.0154)) |
| 244 | +``` |
| 245 | +
|
| 246 | +**RIGHT** |
| 247 | +```go |
| 248 | +log.InfoS(ctx, "Channel open performed", |
| 249 | + slog.Int("user_id", userID), |
| 250 | + btclog.Fmt("amount", "%.8f", 0.00154)) |
| 251 | +``` |
| 252 | +
|
| 253 | +### Wrapping long function definitions |
| 254 | +
|
| 255 | +- If function arguments exceed the 80-character limit, maintain indentation |
| 256 | + on following lines. |
| 257 | +- Do not end a line with an open parenthesis if the function definition is not |
| 258 | + finished. |
| 259 | +
|
| 260 | +**WRONG** |
| 261 | +```go |
| 262 | +func foo(a, b, c, |
| 263 | +) (d, error) { |
| 264 | + |
| 265 | +func bar(a, b, c) ( |
| 266 | + d, error, |
| 267 | +) { |
| 268 | + |
| 269 | +func baz(a, b, c) ( |
| 270 | + d, error) { |
| 271 | +``` |
| 272 | +**RIGHT** |
| 273 | +```go |
| 274 | +func foo(a, b, |
| 275 | + c) (d, error) { |
| 276 | + |
| 277 | +func baz(a, b, c) (d, |
| 278 | + error) { |
| 279 | + |
| 280 | +func longFunctionName( |
| 281 | + a, b, c) (d, error) { |
| 282 | +``` |
| 283 | +
|
| 284 | +- If a function declaration spans multiple lines, the body should start with an |
| 285 | + empty line. |
| 286 | +
|
| 287 | +**WRONG** |
| 288 | +```go |
| 289 | +func foo(a, b, c, |
| 290 | + d, e) error { |
| 291 | + var a int |
| 292 | +} |
| 293 | +``` |
| 294 | +**RIGHT** |
| 295 | +```go |
| 296 | +func foo(a, b, c, |
| 297 | + d, e) error { |
| 298 | + |
| 299 | + var a int |
| 300 | +} |
| 301 | +``` |
| 302 | +
|
| 303 | +## Use of Log Levels |
| 304 | +
|
| 305 | +- Available levels: `trace`, `debug`, `info`, `warn`, `error`, `critical`. |
| 306 | +- Only use `error` for internal errors not triggered by external sources. |
| 307 | +
|
| 308 | +## Testing |
| 309 | +
|
| 310 | +- To run all tests for a specific package: |
| 311 | + `make unit pkg=$pkg` |
| 312 | +- To run a specific test case within a package: |
| 313 | + `make unit pkg=$pkg case=$case` |
| 314 | +
|
| 315 | +## Git Commit Messages |
| 316 | +
|
| 317 | +- **Subject Line:** |
| 318 | + - Format: `subsystem: short description of changes` |
| 319 | + - `subsystem` should be the package primarily affected (e.g., `wallet`, `chain`). |
| 320 | + - For multiple packages, use `+` or `,` as a delimiter (e.g., `wallet+chain`). |
| 321 | + - For widespread changes, use `multi:`. |
| 322 | + - Keep it under 50 characters. |
| 323 | + - Use the present tense (e.g., "Fix bug", not "Fixed bug"). |
| 324 | +
|
| 325 | +- **Message Body:** |
| 326 | + - Separate from the subject with a blank line. |
| 327 | + - Explain the "what" and "why" of the change. |
| 328 | + - Wrap text to 72 characters. |
| 329 | + - Use bullet points for lists. |
0 commit comments