---
tags: code quality
---
# Prysm Code Style Guide
Writing idiomatic Go requires a lot of discipline. If this document aimed to cover all Go best practices, it would be *extremely* long. Fortunately some good soul integrated the [DeepSource static analyzer](https://deepsource.io/) with Prysm's CI pipeline, which makes writing about most practices redundant.
However there are some rules and recommendations which are either not covered by the tool as of the time of writing this paragraph, or that are specific to Prysm. The purpose of the Style Guide is to cover such cases in order to assist team members and contributors while writing code, reviewing pull requests and the like.
### Guide
:scream: - bad code
:heart_eyes: - good code
## Table of Contents
1. [compare errors using `error.Is(err, errType)`](#1-compare-errors-using-errorIserr-errType)
2. [use `%w` instead of `%v` when wrapping errors using `fmt.Errorf`](#2-use-w-instead-of-v-when-wrapping-errors-using-fmtErrorf)
3. [do not use variable names colliding with names of imported packages](#3-do-not-use-variable-names-colliding-with-names-of-imported-packages)
4. [compare strings using `strings.EqualFold()`](#4-compare-strings-using-stringsEqualFold)
5. [do not use prefixes such as `get` in getter function names](#5-do-not-use-prefixes-such-as-get-in-getter-function-names)
6. [name constructors `New` if the package name makes it clear what is being created](#6-name-constructors-New-if-the-package-name-makes-it-clear-what-is-being-created)
7. [name constructors `NewFoo` if the package name does not make it clear what is being created](#7-name-constructors-NewFoo-if-the-package-name-does-not-make-it-clear-what-is-being-created)
8. [create a `log.go` file for each package](#8-create-a-loggo-file-for-each-package)
9. [declare receiver functions on pointer types by default](#9-declare-receiver-functions-on-pointer-types-by-default)
10. [create `HydrateFoo` test helpers](#10-create-HydrateFoo-test-helpers)
11. [do not create files whose name end with `_mock`](#11-do-not-create-files-whose-name-end-with-_mock)
12. [compare byte slices in tests using `assert.DeepEqual`](#12-compare-byte-slices-in-tests-using-assertDeepEqual-and-requireDeepEqual)
### 1. compare errors using `error.Is(err, errType)`
In Golang 1.13 extra method for wrapping errors (see https://golang.org/pkg/errors/ for details) has been introduced. It is generally preferred to use `errors.Is(err, errType)` instead of `err == errType` when comparing non-nil errors to some type as former is able to unwrap wrapped errors and compare correctly.
### :scream:
```go
enr, err := peerStatus.ENR(id)
if err != nil {
if err == peerdata.ErrPeerUnknown {
// Do something
}
}
```
### :heart_eyes:
```go
enr, err := peerStatus.ENR(id)
if err != nil {
if errors.Is(err, peerdata.ErrPeerUnknown) {
// Do something
}
}
```
### 2. use `%w` instead of `%v` when wrapping errors using `fmt.Errorf`
Instead of passing the original error in conventional `fmt.Errorf("sth wrong: %v", err)`, it is better to wrap the error using new verb `%w`, as that way client code can get access to the original error (unwrap) if necessary.
### :scream:
```go
passwordConfirmation, err := PasswordPrompt(confirmText, passwordValidator)
if err != nil {
return "", fmt.Errorf("could not read password confirmation: %v", err)
}
```
### :heart_eyes:
```go
passwordConfirmation, err := PasswordPrompt(confirmText, passwordValidator)
if err != nil {
return "", fmt.Errorf("could not read password confirmation: %w", err)
}
```
### 3. do not use variable names colliding with names of imported packages
Name shadowing is generally considered a bad style. It's sometimes hard to notice that a variable name shadows a package name, especially when the package name is very generic e.g. `db`. Either alias the package import or rename the variable, so that there is no shadowing.
### :scream:
```go
import "github.com/prysmaticlabs/prysm/beacon-chain/db";
(...)
db := testDB.SetupDB(t)
```
### :heart_eyes:
```go
import "github.com/prysmaticlabs/prysm/beacon-chain/db";
(...)
beaconDB := testDB.SetupDB(t)
```
### :heart_eyes:
```go
import beaconDB "github.com/prysmaticlabs/prysm/beacon-chain/db";
(...)
db := testDB.SetupDB(t)
```
### 4. compare strings using `strings.EqualFold()`
When comparing/matching unicode strings in a case-insensitive manner `strings.EqualFold()` should be used (it is more performant than manual turning string into the same case using `strings.ToLower()` or `strings.ToUpper()`, plus it is intended for comparisons)
### :scream:
```go
if strings.ToLower(resp) == "n" {
// Do something
}
```
### :heart_eyes:
```go
if strings.EqualFold(resp, "n") {
// Do something
}
```
### 5. do not use prefixes such as `get` in getter function names
It is confusing to have `fetch`, `get`, `retrieve` as equivalent methods for the same concept of obtaining data. It’s also hard to remember which method prefix uses what key word abstraction for search. Was it `getSignature` or `fetchSignature`? The simplest option is to not use a prefix at all.
### :scream:
```go
func retrieveForkEntry(...
func (f *blocksFetcher) getPeerLock(...
func fetchBlockRootsBySlotRange(...
```
### :heart_eyes:
```go
func forkEntry(...
func (f *blocksFetcher) peerLock(...
func blockRootsBySlotRange(...
```
### 6. name constructors `New` if the package name makes it clear what is being created
If the package name describes the object type being created by a constructor, then the constructor name should be called `New`, to avoid duplication.
### :scream:
```go
package node
func NewNode(cliCtx *cli.Context) (*BeaconNode, error) {
// Do something
}
```
### :heart_eyes:
```go
package node
func New(cliCtx *cli.Context) (*BeaconNode, error) {
// Do something
}
```
### 7. name constructors `NewFoo` if the package name does not make it clear what is being created
If the package name does not indicate the object type created by a constructor, then that constructor's name should include the object type.
### :scream:
```go
package helpers
func New(genesisTime time.Time, secondsPerSlot uint64) *SlotTicker {
// Do something
}
```
### :heart_eyes:
```go
package helpers
func NewSlotTicker(genesisTime time.Time, secondsPerSlot uint64) *SlotTicker {
// Do something
}
```
### 8. create a `log.go` file for each package
Logging is a cross-cutting concern, and as such any high-level changes to it should most likely be applied to the whole codebase. It is therefore helpful to maintain a uniform way of handling package-level loggers. When logging is needed, prepare a `log.go` file in the package which declares a `log` variable. This variable will ensure all logging within the package is done in a predictable way.
### :scream:
```go
// file node.go
package node
import "github.com/sirupsen/logrus"
// prefix has to be repeated in each file
var log = logrus.WithField("prefix", "node")
(...)
log.Error(...
```
### :heart_eyes:
```go
// file log.go
package node
import "github.com/sirupsen/logrus"
// all logging within the package will use the prefix specified in log.go
var log = logrus.WithField("prefix", "node")
```
```go
// file node.go
log.Error(...
```
### 9. declare receiver functions on pointer types by default
Declare methods on `*T` instead of `T` by default, unless you have a strong reason to do otherwise, such as much better performance. See https://dave.cheney.net/tag/receivers for a nice overview of this topic.
### :scream:
```go
func (s ChainService) StateNotifier() statefeed.Notifier { {
// Do something
}
```
### :heart_eyes:
```go
func (s *ChainService) StateNotifier() statefeed.Notifier { {
// Do something
}
```
### 10. create `HydrateFoo` test helpers
fastssz is not forgiving in regards to list size compliance, which results in a lot of array constructions and messy test setups. What is worse is when we have to add a new field in the future, it’ll result in hundreds of line changes in test. To mitigate this issue, add a helper method in `testutil` to hydrate beacon object to comply to fastssz spec.
### :scream:
```go
{
name: "unaggregated only",
atts: []*ethpb.Attestation{
{Data: ðpb.AttestationData{Slot: 1, BeaconBlockRoot: make([]byte, 32), Target: ðpb.Checkpoint{Root: make([]byte, 32)}, Source: ðpb.Checkpoint{Root: make([]byte, 32)}}, Signature: make([]byte, 96)},
{Data: ðpb.AttestationData{Slot: 2, BeaconBlockRoot: make([]byte, 32), Target: ðpb.Checkpoint{Root: make([]byte, 32)}, Source: ðpb.Checkpoint{Root: make([]byte, 32)}}, Signature: make([]byte, 96)},
{Data: ðpb.AttestationData{Slot: 3, BeaconBlockRoot: make([]byte, 32), Target: ðpb.Checkpoint{Root: make([]byte, 32)}, Source: ðpb.Checkpoint{Root: make([]byte, 32)}}, Signature: make([]byte, 96)},
},
count: 3,
},
```
### :heart_eyes:
```go
// helper function in testutil
func HydrateAttestation(a *ethpb.Attestation) *ethpb.Attestation {
if a.Signature == nil {
a.Signature = make([]byte, 96)
}
if a.AggregationBits == nil {
a.AggregationBits = make([]byte, 1)
}
if a.Data == nil {
a.Data = ðpb.AttestationData{}
}
a.Data = HydrateAttestationData(a.Data)
return a
}
```
```go
{
name: "unaggregated only",
atts: []*ethpb.Attestation{
testutil.HydrateAttestation(ðpb.Attestation{Data: ðpb.AttestationData{Slot: 1}}),
testutil.HydrateAttestation(ðpb.Attestation{Data: ðpb.AttestationData{Slot: 2}}),
testutil.HydrateAttestation(ðpb.Attestation{Data: ðpb.AttestationData{Slot: 3}}),
},
count: 3,
},
```
### 11. do not create files whose name end with `_mock`
`.gitattributes` file contains this rule:
```
*_mock.go linguist-generated=true
```
This means that it treats all files ending with `_mock.go` as generated files, and as such ignores the diff, hiding changes to the file on GitHub by default. Do not use the `_mock.go` suffix for file names, as hiding the diff might result in the file being skipped during review.
### :scream:
Bad file name: `powchain_mock`
### :heart_eyes:
Good file name: `mock_powchain`
### 12. compare byte slices in tests using `assert.DeepEqual` and `require.DeepEqual`
Prysm includes its own small library for performing test asserts. The library allows for comparing byte slices - its `assert.DeepEqual`/`require.DeepEqual` functions work in the same way as comparing slices using `bytes.Equal`. Using our library is encouraged because the code becomes more standaradised and is more succinct.
### :scream:
```go
if !bytes.Equal(cachedRoot, newRoot[:]) {
t.Error("Head did not change")
}
```
### :heart_eyes:
```go
assert.DeepEqual(t, cachedRoot, newRoot[:], "Head did not change")
```
Sign in with Wallet
Connect another wallet