build: introduce clang-format in the code base, enforce via CI

.clang-format

@@ -0,0 +1,17 @@
+# Copyright (c) 2023 Bank of Italy
+# Distributed under the GNU AGPLv3 software license, see the accompanying COPYING file.
+
+---
+Language: Cpp
+BasedOnStyle: LLVM
+
+# The rules of the base style above can be explicitly viewed via:
+#     clang-format --style llvm --dump-config
+#
+# The following are the project-specific rules
+ColumnLimit: 110
+InsertBraces: true
+# LineEnding: LF            # requires clang-format >= 16, and ubuntu 22.04 has clang-format 15 at best
+# InsertNewlineAtEOF: true  # requires clang-format >= 16, and ubuntu 22.04 has clang-format 15 at best
+PointerAlignment: Left
+...

.dockerignore

@@ -22,10 +22,12 @@
 #       is explicitly mentioned in the Dockerfile) or will not grab it (if the
 #       file is indirectly referenced through a wildcard).
 
+!/.clang-format
 !/cmake
 !/CMakeLists.txt
 !/engine
 !/infra
+!/Makefile
 !/README.md
 !/scripts
 !/specs

.github/workflows/check-code-formatting.yml

@@ -0,0 +1,27 @@
+name: Check code formatting
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+  # Allows you to run this workflow manually from the Actions tab
+  workflow_dispatch:
+
+jobs:
+  check_format:
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Setup clang-format 15 (we'd like 16, but as of 2023-09-29 it's not there)
+        run: |
+          # TODO: once clang-16 becomes available, please enable the rules that
+          # are currently commented in <BASE>/.clang_format
+          sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-15 10
+          sudo update-alternatives --set clang-format /usr/bin/clang-format-15
+      - name: Checkout itcoin-fbft
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 1
+      - name: check code formatting
+        run: |
+          make check-code-formatting

Dockerfile.fedora38

@@ -7,6 +7,7 @@ FROM fedora:38
 RUN dnf install -y \
     autoconf \
     automake \
+    clang-tools-extra \
     cmake \
     g++ \
     gcc \
@@ -48,6 +49,8 @@ WORKDIR /itcoin-fbft
 
 COPY . /itcoin-fbft
 
+RUN make check-code-formatting
+
 RUN mkdir /itcoin-fbft/build
 
 WORKDIR /itcoin-fbft/build

Dockerfile.ubuntu22.04

@@ -9,6 +9,7 @@ RUN apt update && apt install --no-install-recommends -y \
     automake \
     bsdextrautils \
     ca-certificates \
+    clang-format-15 \
     cmake \
     g++ \
     gcc \
@@ -38,12 +39,18 @@ RUN apt install --no-install-recommends -y \
     libzmq3-dev \
     swi-prolog
 
+# link clang-format-15 as clang-format
+RUN update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-15 10 && \
+    update-alternatives --set clang-format /usr/bin/clang-format-15
+
 RUN mkdir /itcoin-fbft
 
 WORKDIR /itcoin-fbft
 
 COPY . /itcoin-fbft
 
+RUN make check-code-formatting
+
 RUN mkdir /itcoin-fbft/build
 
 WORKDIR /itcoin-fbft/build

Makefile

@@ -0,0 +1,50 @@
+# Copyright (c) 2023 Bank of Italy
+# Distributed under the GNU AGPLv3 software license, see the accompanying COPYING file.
+
+.DEFAULT_GOAL := help
+
+# source: https://stackoverflow.com/questions/18136918/how-to-get-current-relative-directory-of-your-makefile#73509979
+MAKEFILE_ABS_DIR := $(dir $(realpath $(lastword $(MAKEFILE_LIST))))
+
+define PRINT_HELP_PYSCRIPT
+import re, sys
+
+for line in sys.stdin:
+	match = re.match(r'^([0-9a-zA-Z_-]+):.*?## (.*)$$', line)
+	if match:
+		target, help = match.groups()
+		print("%-20s %s" % (target, help))
+endef
+export PRINT_HELP_PYSCRIPT
+
+# source: https://stackoverflow.com/questions/5618615/check-if-a-program-exists-from-a-makefile#25668869
+#
+# please note that there should not be any tabs before the "ifeq/endif"
+# statement
+.PHONY: verify-prerequisites
+verify-prerequisites:
+ifeq (, $(shell command -v clang-format 2> /dev/null))
+	$(error ERROR: please install clang-format)
+endif
+
+.PHONY: check-code-formatting
+# - globstar: enable recursive globbing via "**" in bash >= 4 (equivalent to
+#             shopt -s globstar)
+# - nullglob: when a glob does not match anything, return "" instead of the
+#             literal glob text (equivalent to shopt -s globstar)
+SHELL = /usr/bin/env bash -O globstar -O nullglob
+check-code-formatting: verify-prerequisites ## Check if the code base is correctly formatted. Do not touch the files
+	clang-format --Werror --style=file:.clang-format --ferror-limit=20 --dry-run "${MAKEFILE_ABS_DIR}"/src/*/*.{h,hpp,c,cpp}
+
+.PHONY: reformat-code
+# - globstar: enable recursive globbing via "**" in bash >= 4 (equivalent to
+#             shopt -s globstar)
+# - nullglob: when a glob does not match anything, return "" instead of the
+#             literal glob text (equivalent to shopt -s globstar)
+SHELL = /usr/bin/env bash -O globstar -O nullglob
+reformat-code: verify-prerequisites ## Reformat the code base in the src directory. Can be used as pre-commit hook
+	clang-format --Werror --style=file:.clang-format -i --verbose "${MAKEFILE_ABS_DIR}"/src/**/*.{h,hpp,c,cpp}
+
+.PHONY: help
+help:
+	@python3 -c "$$PRINT_HELP_PYSCRIPT" < $(MAKEFILE_LIST)

README.md

@@ -1,6 +1,7 @@
 # Frosted Byzantine Fault Tolerance (FBFT)
 
 [![tests](https://github.com/bancaditalia/itcoin-fbft/actions/workflows/test-itcoin-fbft.yml/badge.svg?branch=main&event=push)](https://github.com/bancaditalia/itcoin-fbft/actions/workflows/test-itcoin-fbft.yml)
+[![clang-format](https://github.com/bancaditalia/itcoin-fbft/actions/workflows/check-code-formatting.yml/badge.svg?branch=main&event=push)](https://github.com/bancaditalia/itcoin-fbft/actions/workflows/check-code-formatting.yml)
 
 This repository contains the implementation of an itcoin "consensus node",
 which implements the Frosted Byzantine Fault Tolerance (FBFT) Proof-of-Authority consensus algorithm for the **itcoin** blockchain.
@@ -14,12 +15,15 @@ Please note that this software is solely intended for testing and experimentatio
 
 1. Install build and dev dependencies with apt (system-wide):
 
+TODO: add documentation about clang-format in this file
+
 ```
 sudo apt install --no-install-recommends -y \
     autoconf \
     automake \
     bsdextrautils \
     ca-certificates \
+    clang-format-15 \
     cmake \
     g++ \
     gcc \

src/blockchain/BitcoinBlockchain.cpp

@@ -6,140 +6,93 @@
 #include <boost/format.hpp>
 #include <boost/log/trivial.hpp>
 
+#include "../transport/btcclient.h"
 #include "config/FbftConfig.h"
 #include "generate.h"
-#include "../transport/btcclient.h"
 
 using namespace std;
 using namespace itcoin::blockchain;
 
 namespace itcoin {
 namespace blockchain {
 
-BitcoinBlockchain::BitcoinBlockchain(const itcoin::FbftConfig& conf, transport::BtcClient& bitcoind):
-Blockchain(conf), m_bitcoind(bitcoind)
-{
+BitcoinBlockchain::BitcoinBlockchain(const itcoin::FbftConfig& conf, transport::BtcClient& bitcoind)
+    : Blockchain(conf), m_bitcoind(bitcoind) {
   m_reward_address = m_conf.replica_set_v().at(m_conf.id()).p2pkh();
-  BOOST_LOG_TRIVIAL(debug) << str(
-    boost::format("R%1% BitcoinBlockchain, using reward address %2%.")
-      % m_conf.id()
-      % m_reward_address
-  );
+  BOOST_LOG_TRIVIAL(debug) << str(boost::format("R%1% BitcoinBlockchain, using reward address %2%.") %
+                                  m_conf.id() % m_reward_address);
 }
 
-CBlock BitcoinBlockchain::GenerateBlock(uint32_t block_timestamp)
-{
+CBlock BitcoinBlockchain::GenerateBlock(uint32_t block_timestamp) {
   return generateBlock(m_bitcoind, m_reward_address, block_timestamp);
 }
 
-bool BitcoinBlockchain::TestBlockValidity(const uint32_t height, const CBlock& block, bool check_signet_solution)
-{
+bool BitcoinBlockchain::TestBlockValidity(const uint32_t height, const CBlock& block,
+                                          bool check_signet_solution) {
   auto block_ser = HexSerializableCBlock(block);
   const uint32_t block_size_bytes = block_ser.GetHex().length() / 2;
   const std::string block_hash = block.GetBlockHeader().GetHash().ToString();
 
   BOOST_LOG_TRIVIAL(debug) << str(
-    boost::format("R%1% BitcoinBlockchain::TestBlockValidity invoking for "
-    "candidate block at height %2%, blocksize %3% bytes, block hash: %4%")
-      % m_conf.id()
-      % height
-      % block_size_bytes
-      % block_hash
-  );
+      boost::format("R%1% BitcoinBlockchain::TestBlockValidity invoking for "
+                    "candidate block at height %2%, blocksize %3% bytes, block hash: %4%") %
+      m_conf.id() % height % block_size_bytes % block_hash);
 
   Json::Value result;
-  try
-  {
+  try {
     result = m_bitcoind.testblockvalidity(block_ser.GetHex(), check_signet_solution);
-  }
-  catch (jsonrpc::JsonRpcException& e) {
-    BOOST_LOG_TRIVIAL(warning) << str(
-    boost::format("R%1% BitcoinBlockchain::TestBlockValidity for candidate "
-    "block at height %2% with hash %3% raised %4%.")
-      % m_conf.id()
-      % height
-      % block_hash
-      % e.what()
-    );
+  } catch (jsonrpc::JsonRpcException& e) {
+    BOOST_LOG_TRIVIAL(warning) << str(boost::format("R%1% BitcoinBlockchain::TestBlockValidity for candidate "
+                                                    "block at height %2% with hash %3% raised %4%.") %
+                                      m_conf.id() % height % block_hash % e.what());
     return false;
   }
 
   BOOST_LOG_TRIVIAL(debug) << str(
-    boost::format("R%1% BitcoinBlockchain::TestBlockValidity for candidate "
-      "block at height %2% with hash %3%. Result = %4% (null means ok).")
-      % m_conf.id()
-      % height
-      % block_hash
-      % result
-  );
+      boost::format("R%1% BitcoinBlockchain::TestBlockValidity for candidate "
+                    "block at height %2% with hash %3%. Result = %4% (null means ok).") %
+      m_conf.id() % height % block_hash % result);
   return true;
 }
 
-void BitcoinBlockchain::SubmitBlock(const uint32_t height, const CBlock& block)
-{
+void BitcoinBlockchain::SubmitBlock(const uint32_t height, const CBlock& block) {
   const auto block_ser = HexSerializableCBlock(block);
   const std::string block_hash = block.GetBlockHeader().GetHash().ToString();
   try {
     const uint32_t block_size_bytes = block_ser.GetHex().length() / 2;
     BOOST_LOG_TRIVIAL(debug) << str(
-      boost::format(
-        "R%1% BitcoinBlockchain::SubmitBlock submitting block at height %2% "
-        "block size: %3% bytes, block hash: %4%"
-      )
-        % m_conf.id()
-        % height
-        % block_size_bytes
-        % block_hash
-    );
+        boost::format("R%1% BitcoinBlockchain::SubmitBlock submitting block at height %2% "
+                      "block size: %3% bytes, block hash: %4%") %
+        m_conf.id() % height % block_size_bytes % block_hash);
 
     auto result = m_bitcoind.submitblock(block_ser.GetHex());
-    BOOST_LOG_TRIVIAL(debug) << str(
-      boost::format("R%1% BitcoinBlockchain::SubmitBlock for block at height "
-        "%2%, block hash: %3%. Result = %4% (null means ok).")
-        % m_conf.id()
-        % height
-        % block_hash
-        % result
-    );
-  }
-  catch (const jsonrpc::JsonRpcException& e)
-  {
-    if (e.GetMessage() == "The response is invalid: \"duplicate\"\n")
-    {
+    BOOST_LOG_TRIVIAL(debug) << str(boost::format("R%1% BitcoinBlockchain::SubmitBlock for block at height "
+                                                  "%2%, block hash: %3%. Result = %4% (null means ok).") %
+                                    m_conf.id() % height % block_hash % result);
+  } catch (const jsonrpc::JsonRpcException& e) {
+    if (e.GetMessage() == "The response is invalid: \"duplicate\"\n") {
       BOOST_LOG_TRIVIAL(warning) << str(
-        boost::format("R%1% BitcoinBlockchain::SubmitBlock the submitblock "
-        "invocation for block height %2% (hash %3%) failed because the block "
-        "was already in the blockchain. Most probably another replica already "
-        "submitted the same block and was propagated to the local node before "
-        "the submitblock call was attempted.")
-          % m_conf.id()
-          % height
-          % block_hash
-      );
-    }
-    else if (e.GetMessage() == "The response is invalid: \"inconclusive\"\n") {
+          boost::format("R%1% BitcoinBlockchain::SubmitBlock the submitblock "
+                        "invocation for block height %2% (hash %3%) failed because the block "
+                        "was already in the blockchain. Most probably another replica already "
+                        "submitted the same block and was propagated to the local node before "
+                        "the submitblock call was attempted.") %
+          m_conf.id() % height % block_hash);
+    } else if (e.GetMessage() == "The response is invalid: \"inconclusive\"\n") {
       BOOST_LOG_TRIVIAL(warning) << str(
-        boost::format("R%1% BitcoinBlockchain::SubmitBlock the submitblock "
-        "invocation for height %2% (hash %3%) returned 'inconclusive'. This "
-        "problem is temporarily ignored.")
-          % m_conf.id()
-          % height
-          % block_hash
-      );
-    }
-    else {
+          boost::format("R%1% BitcoinBlockchain::SubmitBlock the submitblock "
+                        "invocation for height %2% (hash %3%) returned 'inconclusive'. This "
+                        "problem is temporarily ignored.") %
+          m_conf.id() % height % block_hash);
+    } else {
       BOOST_LOG_TRIVIAL(error) << str(
-        boost::format("R%1% BitcoinBlockchain::SubmitBlock got exception "
-        "while trying to submit block at height %2% (hash %3%): %4%")
-          % m_conf.id()
-          % height
-          % block_hash
-          % e.what()
-      );
+          boost::format("R%1% BitcoinBlockchain::SubmitBlock got exception "
+                        "while trying to submit block at height %2% (hash %3%): %4%") %
+          m_conf.id() % height % block_hash % e.what());
       throw e;
     }
   }
 }
 
-}
-}
+} // namespace blockchain
+} // namespace itcoin

src/blockchain/Blockchain.cpp

@@ -6,10 +6,7 @@
 namespace itcoin {
 namespace blockchain {
 
-Blockchain::Blockchain(const itcoin::FbftConfig& conf):
-m_conf(conf)
-{
-}
+Blockchain::Blockchain(const itcoin::FbftConfig& conf) : m_conf(conf) {}
 
-}
-}
+} // namespace blockchain
+} // namespace itcoin