CREDITS : API CODE REVIEW

Technical analysis and review

GENERAL OVERVIEW

I went through the code since last few days with a critical external eye to give to the team my suggestions and recommendations after an internal audit (compilation, structuration and code review).

The work that I have done is enterily to give better feedback to the community and to the core development team. I was not paid to do this review and it’s my duty as technical advisor of the project to give you publicly my thoughts.

First of all, the following analysis will show you the modelisation of the code to better figure out the structuration of the API.

The API module of CREDITS platform is already avalaible to the CREDITS github account and included the following files :

APIHandler.cpp

APIHandler.h

APIHandlerBase.cpp

APIHandlerBase.h

APIHandlerInterface.h

CallStats.cpp

CallStats.h

csconnector.cpp

DBHandlers.cpp

DBHandlers.h

DebugLog.h

Validation.h

ValidationTests.cpp

The state of the actual code shows the non presence of a licence within a code headers files. I know that it’s not a final version but I strongly recommend you to add it as soon as possible.

I noticed that major party of the comments in your code is in Russian language. In the objective to increase readiness I suggest you to translate it in English, the Shakespeare language. However I would loved to read it in Moliere language.

By the way as an API server, the code will primarily be run and compile on Linux platform. Based on this fact I didn’t found any Makefile to help me to do it properly. I strongly advise you to create a Makefile which is used to build binary programs from source code and make your workflow reproducible.

For the validation part, the structure of the code is implemented as macros. While this implementation gives great adaptability, it sacrifices a lot of understandability and readiness. Using a standard test library could be a good start if you plan to change it.

You can also separate unit test from “prod” files. It allows contributors to add their own tests during the test period that you have planning to run in the coming days.

Deep dive into each files

I analyzed each of the presented files separately and modelized it to give as much as possible details of the architecture chosen by the CREDITS core team.

APIHandler.cpp

• #include “APIHandler.h”
• #include “DebugLog.h”
• #include “csconnector/csconnector.h”
• #include “CallStats.h”
• #include “Validation.h”

The graph of the header files looks as follows:

Namespace:

• csconnector
• csconnector::detail

Functions:

• VALIDATE(source)
• VALIDATE(target)
• VALIDATE(currency)

APIHandler.h

#include <mutex>

#include “APIHandlerInterface.h”

The graph of the included header files looks as follows:

The graph of the files to which this file is included:

Classes:

class csconnector::APIHandler

Namespace:

csconnector

Remark:

The file header describes the methods and their parameters. There are no other relevant remarks that I currently noticed.

APIHandlerBase.cpp

#include “APIHandlerBase.h”

The graph of the included header files looks as follows:

Classes:

struct csconnector::detail::APIRequestStatus

Namespace:

csconnector

csconnector::detail

Variables:

APIRequestStatus csconnector::detail::statuses [static_cast< size_t >(APIHandlerBase::APIRequestStatusType::MAX)]

Remark:

Correct application of data types for the reduction of the used memory size when assigning the status of a function call.

APIHandlerBase.h

#include “API.h”

The graph of the included header files for APIHandlerBase.h:

The graph of files, which this file is included in:

Classes:

struct csconnector::APIHandlerBase

Namespace:

csconnector

Remarks:

The file header describes the methods and their parameters. There are no relevant remarks.

APIHandlerInterface.h

#include “API.h”
#include “APIHandlerBase.h”

The graph of the included header files for APIHandlerBase.h:

The graph of files, which this file is included in:

Classes:

struct csconnector::APIHandlerInterface

Namespace:

сsconnector

Remark:

The file header describes the methods and their parameters. There are no relevant remarks.

CallStats.cpp

#include “CallStats.h”
#include <atomic>
#include <array>
#include <thread>
#include “DebugLog.h”

The graph of the included header files for CallStats.cpp:

Type definitions:

• using csconnector::call_stats::Counter = std::atomic<int>
• using csconnector::call_stats::NumCallsPerCommand = std::array<Counter, NumCommands>
• using csconnector::call_stats::NumCallsPerCommandStats = std::array<int, NumCommands>

Functions:

NumCallsPerCommandStats csconnector::call_stats::get ()

void csconnector::call_stats::count (Commands command)

void csconnector::call_stats::clear ()

void csconnector::call_stats::start ()

void csconnector::call_stats::stop ()

Variables:

• constexpr size_t csconnector::call_stats::NumCommands = (size_t)Commands::Max
• NumCallsPerCommand csconnector::call_stats::numCallsPerCommand
• std::chrono::steady_clock::time_point csconnector::call_stats::lastUpdateTime = std::chrono::steady_clock::now()
• std::thread csconnector::call_stats::thread
• std::atomic_bool csconnector::call_stats::quit {false}

Remarks:

CallStats.h

#include <csconnector/csconnector.h>

The graph of the included header files for CallStats.cpp:

The graph of files, which this file is included in:

Namespace:

csconnector

csconnector::call_stats

Functions:

void csconnector::call_stats::start ()

void csconnector::call_stats::stop ()

void csconnector::call_stats::count (Commands command)

Remarks:

The file header describes the methods and their parameters. There are no relevant remarks.

csconnector.cpp

#include “csconnector/csconnector.h”

#include “DebugLog.h”

#include “APIHandler.h”

#include “DBHandlers.h”

#include “CallStats.h”

#include <thread>

#include <memory>

#include <thrift/protocol/TBinaryProtocol.h>

#include <thrift/transport/TServerSocket.h>

#include <thrift/transport/TBufferTransports.h>

#include <thrift/server/TThreadedServer.h>

The graph of the included header files for csconnector.cpp:

Namespace:

csconnector

csconnector::detail

Type definitions:

typedef std::lock_guard< std::mutex > csconnector::detail::ScopedLock

Functions:

• void csconnector::detail::start (const Config &config)
• void csconnector::detail::stop ()
• void csconnector::start (const Config &config)
• void csconnector::stop ()

Variables:

• td::unique_ptr< TThreadedServer > csconnector::detail::server = nullptr
• std::mutex csconnector::detail::mutex
• std::thread csconnector::detail::thread

Remarks:

DBHandlers.cpp

#include “DBHandlers.h”

#include <csdb/csdb.h>

#include “csconnector/csconnector.h”

#include <algorithm>

#include <cstring>

The graph of the included header files for DBHandlers.cpp:

Namespace:

db_handlers

Type definitions:

using db_handlers::PoolNumber = uint64_t

Functions:

• void db_handlers::init ()
• void db_handlers::deinit ()
• void db_handlers::BalanceGet (BalanceGetResult &_return, const Address &address, const Currency &currency)
• void db_handlers::string_to_uuid (const std::string &uuid_str, uuid_t uuid)
• bool db_handlers::GetTransaction (const TransactionId &transactionId, Transaction &transaction)
• void db_handlers::TransactionGet (TransactionGetResult &_return, const TransactionId &transactionId)
• void db_handlers::SubstituteTransactionHash (api::Transaction &transaction, const std::string &newHash)
• Thats actually a hack.
• std::string db_handlers::FormatTransactionHash (const std::string &poolHash, size_t transacionNumber)
• void db_handlers::TransactionsGet (TransactionsGetResult &_return, const Address &address, const int64_t offset, const int64_t limit)
• void db_handlers::PoolListGet (PoolListGetResult &_return, const int64_t offset, const int64_t limit)
• void db_handlers::PoolGet (PoolGetResult &_return, const PoolHash &hash)

Remarks:

DBHandlers.h

#include “API.h”

The graph of the included header files for DBHandlers.h:

The graph of files, which this file is included in:

Namespace:

db_handlers

Functions:

• void db_handlers::init ()
• void db_handlers::deinit ()
• void db_handlers::BalanceGet (api::BalanceGetResult &_return, const api::Address &address, const api::Currency &currency)
• void db_handlers::TransactionGet (api::TransactionGetResult &_return, const api::TransactionId &transactionId)
• void db_handlers::TransactionsGet (api::TransactionsGetResult &_return, const api::Address &address, const int64_t offset, const int64_t limit)
• void db_handlers::PoolListGet (api::PoolListGetResult &_return, const int64_t offset, const int64_t limit)
• void db_handlers::PoolGet (api::PoolGetResult &_return, const api::PoolHash &hash)

Remark:

The file header describes the methods and their parameters. There are no relevant remarks.

DebugLog.h

#include <iostream>

The graph of the included header files for DebugLog.h:

The graph of files, which this file is included in:

Macros:

#define DEBUG_LOG

Functions:

• void Log ()
• template<typename T , typename… Args>
• void Log (T t, Args… args)
• template<typename T , typename… Args>
• void DebugLog (T t, Args… args)

Macros:

DEBUG_LOG

#define DEBUG_LOG

Functions:

DebugLog()

template <typename T , typename… Args>

void DebugLog ( T t,Args… args)

Call graph:

The graph of the function call:

Log() [1/2]

void Log

The graph of the function call:

Log() [2/2]

template<typename T , typename… Args>

void Log(T t,

Args… args

)

Call graph:

Remarks:

There are no relevant remarks at thsi stage of the development.

Validation.h

#include <string>

#include <vector>

#include <memory>

#include <tuple>

#include <functional>

The graph of the included header files for Validation.h:

The graph of files, which this file is included in:

Classes:

struct validation::Validator< T >

struct validation::StringLengthValidator< O >

struct validation::StringLengthValidator< O >::Int2Type< I >

struct validation::NonEmptyValidator

struct validation::NonZeroValidator

struct validation::EqualToValidator< T >

struct validation::CustomValidator< T >

struct validation::ValidatorBuilder< T >

struct validation::Dummy

struct validation::ValidationTraits< T, D >

Namespace:

validation

Macros:

#define VALIDATE_BEGIN_(C, D)

#define VALIDATE_BEGIN(C) VALIDATE_BEGIN_(C, Dummy)

#define VALIDATE_BEGIN_EX(C, D) struct D{}; VALIDATE_BEGIN_(C, D)

#define VALIDATE(F)

#define VALIDATE_END()

Type definition:

using validation::FieldId = uint8_t

using validation::ValidatorId = uint8_t

using validation::ValidationResult = std::tuple< FieldId, ValidatorId >

Enumerations:

enum validation::StringLengthOperation { validation::Less, validation::More, validation::Equal }

Variables:

constexpr ValidationResult validation::NoError = { 0, 0 }

Macros:

VALIDATE

#define VALIDATE ( F )

Macro definition:

Remarks:

The file header describes the methods and their parameters. There are no relevant remarks at this stage of the development.

ValidationTests.cpp

#include “Validation.h”

#include <assert.h>

The graph of the included header files for ValidationTests.cpp:

Classes:

struct Test

Functions:

VALIDATE (x).EqualTo(1)

VALIDATE (s).Length< More >(2)

void validationTest ()

Functions:

VALIDATE() [1/2]

VALIDATE ( x )

VALIDATE() [2/2]

VALIDATE ( s )

validationTest()

void validationTest ()

The graph of the function call:

Remarks:

CONCLUSION

As noticed some improvements into the code still necessary but the structuration of the code seems to be design to be scalable. As technical advisor of Credits, I feel confident on the project and what the team is trying to achieve.

Indeed, building a new protocol from scratch behind a strong community takes time. The core team is going to publish part by part the code on GitHub in order to involve the community members to receive feedbacks. CREDITS team has already started to share a part of the code and they will continue to do it in the coming days.

The community will be involved more and more when a stable version is ready to be deployed. Personally, I rarely saw in the blockchain community space a team as close as their community like Credits.

Patience leads to salvation, rush to misery. Who wants to do great things must think deeply about the details because evil is behind. Be patient the revolution is coming soon.