Skip to content

Commit

Permalink
Properly wrap errors (ethereum-optimism#3366)
Browse files Browse the repository at this point in the history
* op-node: Properly wrap errors

* op-proposer: Properly wrap errors

* Update linters to use errorlint

This includes a hack with golangci-lint to disable the assertion
and comparison check in errorlint but to retain the errorf lint.

In the the future the assertion lint could probably be fixed, but
the comparison lint may be overactive with certain cases.
  • Loading branch information
trianglesphere authored Sep 9, 2022
1 parent c564e4e commit abaa065
Show file tree
Hide file tree
Showing 22 changed files with 81 additions and 81 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ jobs:
fi
- run:
name: Lint
command: golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell ./...
command: golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint -e "errors.As" -e "errors.Is" ./...
working_directory: <<parameters.working_directory>>
- store_test_results:
path: /test-results
Expand Down
2 changes: 1 addition & 1 deletion op-batcher/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test:
go test -v ./...

lint:
golangci-lint run -E asciicheck,goimports,misspell ./...
golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint -e "errors.As" -e "errors.Is"

.PHONY: \
op-batcher \
Expand Down
2 changes: 1 addition & 1 deletion op-e2e/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ test:
go test -v ./...

lint:
golangci-lint run -E asciicheck,goimports,misspell ./...
golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint -e "errors.As" -e "errors.Is"

.PHONY: \
test \
Expand Down
2 changes: 1 addition & 1 deletion op-node/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test:
go test -v ./...

lint:
golangci-lint run -E asciicheck,goimports,misspell ./...
golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint -e "errors.As" -e "errors.Is"

fuzz:
go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzExecutionPayloadUnmarshal ./eth
Expand Down
6 changes: 3 additions & 3 deletions op-node/eth/account_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ func (res *AccountResult) Verify(stateRoot common.Hash) error {
accountClaimed := []interface{}{uint64(res.Nonce), (*big.Int)(res.Balance).Bytes(), res.StorageHash, res.CodeHash}
accountClaimedValue, err := rlp.EncodeToBytes(accountClaimed)
if err != nil {
return fmt.Errorf("failed to encode account from retrieved values: %v", err)
return fmt.Errorf("failed to encode account from retrieved values: %w", err)
}

// create a db with all trie nodes
db := memorydb.New()
for i, encodedNode := range res.AccountProof {
nodeKey := crypto.Keccak256(encodedNode)
if err := db.Put(nodeKey, encodedNode); err != nil {
return fmt.Errorf("failed to load proof value %d into mem db: %v", i, err)
return fmt.Errorf("failed to load proof value %d into mem db: %w", i, err)
}
}

Expand All @@ -54,7 +54,7 @@ func (res *AccountResult) Verify(stateRoot common.Hash) error {
// now get the full value from the account proof, and check that it matches the JSON contents
accountProofValue, err := proofTrie.TryGet(key[:])
if err != nil {
return fmt.Errorf("failed to retrieve account value: %v", err)
return fmt.Errorf("failed to retrieve account value: %w", err)
}

if !bytes.Equal(accountClaimedValue, accountProofValue) {
Expand Down
2 changes: 1 addition & 1 deletion op-node/eth/ssz.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (payload *ExecutionPayload) UnmarshalSSZ(scope uint32, r io.Reader) error {
copy(payload.ExtraData, buf[extraDataOffset:transactionsOffset])
txs, err := unmarshalTransactions(buf[transactionsOffset:])
if err != nil {
return fmt.Errorf("failed to unmarshal transactions list: %v", err)
return fmt.Errorf("failed to unmarshal transactions list: %w", err)
}
payload.Transactions = txs
return nil
Expand Down
2 changes: 1 addition & 1 deletion op-node/eth/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func BlockAsPayload(bl *types.Block) (*ExecutionPayload, error) {
for i, tx := range bl.Transactions() {
otx, err := tx.MarshalBinary()
if err != nil {
return nil, fmt.Errorf("tx %d failed to marshal: %v", i, err)
return nil, fmt.Errorf("tx %d failed to marshal: %w", i, err)
}
opaqueTxs[i] = otx
}
Expand Down
2 changes: 1 addition & 1 deletion op-node/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (n *OpNode) initL1(ctx context.Context, cfg *Config) error {
client.NewInstrumentedRPC(l1Node, n.metrics), n.log, n.metrics.L1SourceCache,
sources.L1ClientDefaultConfig(&cfg.Rollup, trustRPC))
if err != nil {
return fmt.Errorf("failed to create L1 source: %v", err)
return fmt.Errorf("failed to create L1 source: %w", err)
}

// Keep subscribed to the L1 heads, which keeps the L1 maintainer pointing to the best headers to sync
Expand Down
40 changes: 20 additions & 20 deletions op-node/p2p/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,20 @@ func NewConfig(ctx *cli.Context) (*Config, error) {

p, err := loadNetworkPrivKey(ctx)
if err != nil {
return nil, fmt.Errorf("failed to load p2p priv key: %v", err)
return nil, fmt.Errorf("failed to load p2p priv key: %w", err)
}
conf.Priv = p

if err := conf.loadListenOpts(ctx); err != nil {
return nil, fmt.Errorf("failed to load p2p listen options: %v", err)
return nil, fmt.Errorf("failed to load p2p listen options: %w", err)
}

if err := conf.loadDiscoveryOpts(ctx); err != nil {
return nil, fmt.Errorf("failed to load p2p discovery options: %v", err)
return nil, fmt.Errorf("failed to load p2p discovery options: %w", err)
}

if err := conf.loadLibp2pOpts(ctx); err != nil {
return nil, fmt.Errorf("failed to load p2p options: %v", err)
return nil, fmt.Errorf("failed to load p2p options: %w", err)
}

conf.ConnGater = DefaultConnGater
Expand All @@ -189,11 +189,11 @@ func (conf *Config) loadListenOpts(ctx *cli.Context) error {
var err error
conf.ListenTCPPort, err = validatePort(ctx.GlobalUint(flags.ListenTCPPort.Name))
if err != nil {
return fmt.Errorf("bad listen TCP port: %v", err)
return fmt.Errorf("bad listen TCP port: %w", err)
}
conf.ListenUDPPort, err = validatePort(ctx.GlobalUint(flags.ListenUDPPort.Name))
if err != nil {
return fmt.Errorf("bad listen UDP port: %v", err)
return fmt.Errorf("bad listen UDP port: %w", err)
}
return nil
}
Expand All @@ -206,17 +206,17 @@ func (conf *Config) loadDiscoveryOpts(ctx *cli.Context) error {
var err error
conf.AdvertiseTCPPort, err = validatePort(ctx.GlobalUint(flags.AdvertiseTCPPort.Name))
if err != nil {
return fmt.Errorf("bad advertised TCP port: %v", err)
return fmt.Errorf("bad advertised TCP port: %w", err)
}
conf.AdvertiseUDPPort, err = validatePort(ctx.GlobalUint(flags.AdvertiseUDPPort.Name))
if err != nil {
return fmt.Errorf("bad advertised UDP port: %v", err)
return fmt.Errorf("bad advertised UDP port: %w", err)
}
adIP := ctx.GlobalString(flags.AdvertiseIP.Name)
if adIP != "" { // optional
ips, err := net.LookupIP(adIP)
if err != nil {
return fmt.Errorf("failed to lookup IP of %q to advertise in ENR: %v", adIP, err)
return fmt.Errorf("failed to lookup IP of %q to advertise in ENR: %w", adIP, err)
}
// Find the first v4 IP it resolves to
for _, ip := range ips {
Expand All @@ -239,7 +239,7 @@ func (conf *Config) loadDiscoveryOpts(ctx *cli.Context) error {
}
conf.DiscoveryDB, err = enode.OpenDB(dbPath)
if err != nil {
return fmt.Errorf("failed to open discovery db: %v", err)
return fmt.Errorf("failed to open discovery db: %w", err)
}

records := strings.Split(ctx.GlobalString(flags.Bootnodes.Name), ",")
Expand All @@ -250,7 +250,7 @@ func (conf *Config) loadDiscoveryOpts(ctx *cli.Context) error {
}
nodeRecord, err := enode.Parse(enode.ValidSchemes, recordB64)
if err != nil {
return fmt.Errorf("bootnode record %d (of %d) is invalid: %q err: %v", i, len(records), recordB64, err)
return fmt.Errorf("bootnode record %d (of %d) is invalid: %q err: %w", i, len(records), recordB64, err)
}
conf.Bootnodes = append(conf.Bootnodes, nodeRecord)
}
Expand Down Expand Up @@ -300,7 +300,7 @@ func (conf *Config) loadLibp2pOpts(ctx *cli.Context) error {
}
a, err := multiaddr.NewMultiaddr(addr)
if err != nil {
return fmt.Errorf("failed to parse multi addr of static peer %d (out of %d): %q err: %v", i, len(addrs), addr, err)
return fmt.Errorf("failed to parse multi addr of static peer %d (out of %d): %q err: %w", i, len(addrs), addr, err)
}
conf.StaticPeers = append(conf.StaticPeers, a)
}
Expand All @@ -318,7 +318,7 @@ func (conf *Config) loadLibp2pOpts(ctx *cli.Context) error {
return fmt.Errorf("could not recognize mux %s", v)
}
if err != nil {
return fmt.Errorf("failed to make %s constructor: %v", v, err)
return fmt.Errorf("failed to make %s constructor: %w", v, err)
}
conf.HostMux = append(conf.HostMux, mc)
}
Expand All @@ -342,7 +342,7 @@ func (conf *Config) loadLibp2pOpts(ctx *cli.Context) error {
return fmt.Errorf("could not recognize security %s", v)
}
if err != nil {
return fmt.Errorf("failed to make %s constructor: %v", v, err)
return fmt.Errorf("failed to make %s constructor: %w", v, err)
}
conf.HostSecurity = append(conf.HostSecurity, sc)
}
Expand All @@ -368,7 +368,7 @@ func (conf *Config) loadLibp2pOpts(ctx *cli.Context) error {
} else {
store, err = leveldb.NewDatastore(peerstorePath, nil) // default leveldb options are fine
if err != nil {
return fmt.Errorf("failed to open leveldb db for peerstore: %v", err)
return fmt.Errorf("failed to open leveldb db for peerstore: %w", err)
}
}
conf.Store = store
Expand All @@ -386,26 +386,26 @@ func loadNetworkPrivKey(ctx *cli.Context) (*crypto.Secp256k1PrivateKey, error) {
if os.IsNotExist(err) {
p, _, err := crypto.GenerateSecp256k1Key(rand.Reader)
if err != nil {
return nil, fmt.Errorf("failed to generate new p2p priv key: %v", err)
return nil, fmt.Errorf("failed to generate new p2p priv key: %w", err)
}
b, err := p.Raw()
if err != nil {
return nil, fmt.Errorf("failed to encode new p2p priv key: %v", err)
return nil, fmt.Errorf("failed to encode new p2p priv key: %w", err)
}
f, err := os.OpenFile(keyPath, os.O_CREATE|os.O_WRONLY, 0600)
if err != nil {
return nil, fmt.Errorf("failed to store new p2p priv key: %v", err)
return nil, fmt.Errorf("failed to store new p2p priv key: %w", err)
}
defer f.Close()
if _, err := f.WriteString(hex.EncodeToString(b)); err != nil {
return nil, fmt.Errorf("failed to write new p2p priv key: %v", err)
return nil, fmt.Errorf("failed to write new p2p priv key: %w", err)
}
return (p).(*crypto.Secp256k1PrivateKey), nil
} else {
defer f.Close()
data, err := io.ReadAll(f)
if err != nil {
return nil, fmt.Errorf("failed to read priv key file: %v", err)
return nil, fmt.Errorf("failed to read priv key file: %w", err)
}
return parsePriv(strings.TrimSpace(string(data)))
}
Expand Down
10 changes: 5 additions & 5 deletions op-node/p2p/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func enrToAddrInfo(r *enode.Node) (*peer.AddrInfo, *crypto.Secp256k1PublicKey, e
}
mAddr, err := multiaddr.NewMultiaddr(fmt.Sprintf("/%s/%s/tcp/%d", ipScheme, ip.String(), r.TCP()))
if err != nil {
return nil, nil, fmt.Errorf("could not construct multi addr: %v", err)
return nil, nil, fmt.Errorf("could not construct multi addr: %w", err)
}
var enrPub Secp256k1
if err := r.Load(&enrPub); err != nil {
Expand All @@ -146,7 +146,7 @@ func enrToAddrInfo(r *enode.Node) (*peer.AddrInfo, *crypto.Secp256k1PublicKey, e
pub := (*crypto.Secp256k1PublicKey)(&enrPub)
peerID, err := peer.IDFromPublicKey(pub)
if err != nil {
return nil, pub, fmt.Errorf("could not compute peer ID from pubkey for multi-addr: %v", err)
return nil, pub, fmt.Errorf("could not compute peer ID from pubkey for multi-addr: %w", err)
}
return &peer.AddrInfo{
ID: peerID,
Expand Down Expand Up @@ -177,18 +177,18 @@ func (o *OptimismENRData) EncodeRLP(w io.Writer) error {
func (o *OptimismENRData) DecodeRLP(s *rlp.Stream) error {
b, err := s.Bytes()
if err != nil {
return fmt.Errorf("failed to decode outer ENR entry: %v", err)
return fmt.Errorf("failed to decode outer ENR entry: %w", err)
}
// We don't check the byte length: the below readers are limited, and the ENR itself has size limits.
// Future "optimism" entries may contain additional data, and will be tagged with a newer version etc.
r := bytes.NewReader(b)
chainID, err := binary.ReadUvarint(r)
if err != nil {
return fmt.Errorf("failed to read chain ID var int: %v", err)
return fmt.Errorf("failed to read chain ID var int: %w", err)
}
version, err := binary.ReadUvarint(r)
if err != nil {
return fmt.Errorf("failed to read version var int: %v", err)
return fmt.Errorf("failed to read version var int: %w", err)
}
o.chainID = chainID
o.version = version
Expand Down
14 changes: 7 additions & 7 deletions op-node/p2p/gossip.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config) pubsub.ValidatorEx
// uint64 -> *seenBlocks
blockHeightLRU, err := lru.New(100)
if err != nil {
panic(fmt.Errorf("failed to set up block height LRU cache: %v", err))
panic(fmt.Errorf("failed to set up block height LRU cache: %w", err))
}

return func(ctx context.Context, id peer.ID, message *pubsub.Message) pubsub.ValidationResult {
Expand Down Expand Up @@ -330,13 +330,13 @@ func (p *publisher) PublishL2Payload(ctx context.Context, payload *eth.Execution

buf.Write(make([]byte, 65))
if _, err := payload.MarshalSSZ(buf); err != nil {
return fmt.Errorf("failed to encoded execution payload to publish: %v", err)
return fmt.Errorf("failed to encoded execution payload to publish: %w", err)
}
data := buf.Bytes()
payloadData := data[65:]
sig, err := signer.Sign(ctx, SigningDomainBlocksV1, p.cfg.L2ChainID, payloadData)
if err != nil {
return fmt.Errorf("failed to sign execution payload with signer: %v", err)
return fmt.Errorf("failed to sign execution payload with signer: %w", err)
}
copy(data[:65], sig[:])

Expand All @@ -359,15 +359,15 @@ func JoinGossip(p2pCtx context.Context, self peer.ID, ps *pubsub.PubSub, log log
pubsub.WithValidatorTimeout(3*time.Second),
pubsub.WithValidatorConcurrency(4))
if err != nil {
return nil, fmt.Errorf("failed to register blocks gossip topic: %v", err)
return nil, fmt.Errorf("failed to register blocks gossip topic: %w", err)
}
blocksTopic, err := ps.Join(blocksTopicName)
if err != nil {
return nil, fmt.Errorf("failed to join blocks gossip topic: %v", err)
return nil, fmt.Errorf("failed to join blocks gossip topic: %w", err)
}
blocksTopicEvents, err := blocksTopic.EventHandler()
if err != nil {
return nil, fmt.Errorf("failed to create blocks gossip topic handler: %v", err)
return nil, fmt.Errorf("failed to create blocks gossip topic handler: %w", err)
}
go LogTopicEvents(p2pCtx, log.New("topic", "blocks"), blocksTopicEvents)

Expand All @@ -379,7 +379,7 @@ func JoinGossip(p2pCtx context.Context, self peer.ID, ps *pubsub.PubSub, log log

subscription, err := blocksTopic.Subscribe()
if err != nil {
return nil, fmt.Errorf("failed to subscribe to blocks gossip topic: %v", err)
return nil, fmt.Errorf("failed to subscribe to blocks gossip topic: %w", err)
}

subscriber := MakeSubscriber(log, BlocksHandler(gossipIn.OnUnsafeL2Payload))
Expand Down
18 changes: 9 additions & 9 deletions op-node/p2p/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,40 +48,40 @@ func (conf *Config) Host(log log.Logger) (host.Host, error) {
pub := conf.Priv.GetPublic()
pid, err := peer.IDFromPublicKey(pub)
if err != nil {
return nil, fmt.Errorf("failed to derive pubkey from network priv key: %v", err)
return nil, fmt.Errorf("failed to derive pubkey from network priv key: %w", err)
}

ps, err := pstoreds.NewPeerstore(context.Background(), conf.Store, pstoreds.DefaultOpts())
if err != nil {
return nil, fmt.Errorf("failed to open peerstore: %v", err)
return nil, fmt.Errorf("failed to open peerstore: %w", err)
}

if err := ps.AddPrivKey(pid, conf.Priv); err != nil {
return nil, fmt.Errorf("failed to set up peerstore with priv key: %v", err)
return nil, fmt.Errorf("failed to set up peerstore with priv key: %w", err)
}
if err := ps.AddPubKey(pid, pub); err != nil {
return nil, fmt.Errorf("failed to set up peerstore with pub key: %v", err)
return nil, fmt.Errorf("failed to set up peerstore with pub key: %w", err)
}

connGtr, err := conf.ConnGater(conf)
if err != nil {
return nil, fmt.Errorf("failed to open connection gater: %v", err)
return nil, fmt.Errorf("failed to open connection gater: %w", err)
}

connMngr, err := conf.ConnMngr(conf)
if err != nil {
return nil, fmt.Errorf("failed to open connection manager: %v", err)
return nil, fmt.Errorf("failed to open connection manager: %w", err)
}

listenAddr, err := addrFromIPAndPort(conf.ListenIP, conf.ListenTCPPort)
if err != nil {
return nil, fmt.Errorf("failed to make listen addr: %v", err)
return nil, fmt.Errorf("failed to make listen addr: %w", err)
}
tcpTransport, err := lconf.TransportConstructor(
tcp.NewTCPTransport,
tcp.WithConnectionTimeout(time.Minute*60)) // break unused connections
if err != nil {
return nil, fmt.Errorf("failed to create TCP transport: %v", err)
return nil, fmt.Errorf("failed to create TCP transport: %w", err)
}
// TODO: technically we can also run the node on websocket and QUIC transports. Maybe in the future?

Expand Down Expand Up @@ -140,7 +140,7 @@ func (conf *Config) Host(log log.Logger) (host.Host, error) {
for _, peerAddr := range conf.StaticPeers {
addr, err := peer.AddrInfoFromP2pAddr(peerAddr)
if err != nil {
return nil, fmt.Errorf("bad peer address: %v", err)
return nil, fmt.Errorf("bad peer address: %w", err)
}
h.Peerstore().AddAddrs(addr.ID, addr.Addrs, time.Hour*24*7)
// We protect the peer, so the connection manager doesn't decide to prune it.
Expand Down
Loading

0 comments on commit abaa065

Please sign in to comment.