# Stonks or not stonks\. Checking Lean trading engine source code

These days, many people take an interest in stocks, bonds, and exchanges, with some even managing their own investment portfolios \(and we're not talking CVs here\)\. Numerous technologies and tools exist to automate trading\. Imagine potential losses that errors in such software source code could cause\. That's why we'll review potential bugs in the popular Lean trading engine\.

![1271_Lean_errors/image1.png](https://import.viva64.com/docx/blog/1271_Lean_errors/image1.png)

## Introduction

What is Lean? [QuantConnect Lean](https://github.com/QuantConnect/Lean/) is an open\-source algorithmic trading engine designed for easy strategy research, back testing, and live trading\. It runs on Windows, Linux, and macOS, integrating with common data providers and brokerages for rapid deployment of algorithmic trading strategies\.

This is a high\-stakes project where any error can lead to significant financial losses\. Therefore, developing such tools demands rigorous testing and code analysis means\. [The PVS\-Studio static analyzer](https://pvs-studio.com/en/pvs-studio/try-free/) can serve as a valuable aid in this development\.

We've previously written about errors in this project \("[Talking About Errors in the QuantConnect Lean Code](https://pvs-studio.com/en/blog/posts/csharp/0782/)"\) quite some time ago\. We regularly add new diagnostic rules to the analyzer and have enhanced existing ones\. Lean has also evolved continuously, so it's interesting to see what's changed in the meantime\.

## Checking the project

### Warnings issued by new diagnostic rules

Let's start with several warnings from new diagnostic rules that didn't exist at the time of our previous review\.

**Snippet 1**

```cpp
public override int GetHashCode()
{
  unchecked
  {
    var hashCode = Definition.GetHashCode();
    var arr = new int[Legs.Count];
    for (int i = 0; i < Legs.Count; i++)
    {
      arr[i] = Legs[i].GetHashCode();
    }

    Array.Sort(arr);

    for (int i = 0; i < arr.Length; i++)
    {
      hashCode = (hashCode * 397) ^ arr[i];
    }

    return hashCode;
  }
}

public override bool Equals(object obj)
{
    ....

    return Equals((OptionStrategyDefinitionMatch) obj);
}
```

The PVS\-Studio warning: [V3192](https://pvs-studio.com/en/docs/warnings/v3192/) The 'Legs' property is used in the 'GetHashCode' method but is missing from the 'Equals' method\. OptionStrategyDefinitionMatch\.cs 176

The analyzer performed an [interprocedural analysis](https://pvs-studio.com/en/blog/terms/6785/) of the `Equals` method, which calls an overridden `Equals` method, and detected that the `Legs` property wasn't used despite being referenced in `GetHashCode`\.

Look at the `Equals` method carefully:

```cpp
public bool Equals(OptionStrategyDefinitionMatch other)
{
  ....

  var positions = other.Legs
                        .ToDictionary(leg => leg.Position, 
                                       leg => leg.Multiplier);
  foreach (var leg in other.Legs)                                   // <=
  {
    int multiplier;
    if (!positions.TryGetValue(leg.Position, out multiplier))
    {
      return false;
    }

    if (leg.Multiplier != multiplier)
    {
      return false;
    }
  }

  return true;
}
```

Note that it iterates through `other.Legs`\. For each element in this collection, we attempt to locate it in the `positions` dictionary—but this dictionary derives from `other.Legs`\. Essentially, the code checks whether elements exist in the same collection they originate from\.

The fix is straightforward: replace `other.Legs` with `Legs` at the marked location\.

**Snippet 2**

```cpp
public void FutureMarginModel_MarginEntriesValid(string market)
{
  ....
  var lineNumber = 0;
  var errorMessageTemplate = $"Error encountered in file 
                               {marginFile.Name} on line ";
  var csv = File.ReadLines(marginFile.FullName)
                .Where(x =>    !x.StartsWithInvariant("#") 
                            && !string.IsNullOrWhiteSpace(x))
                .Skip(1)
                .Select(x =>
  {
    lineNumber++;                                                  // <=

    ....
  });

  lineNumber = 0;                                                  // <=
  foreach (var line in csv)
  {
    lineNumber++;                                                  // <=
    ....
  }
}
```

The PVS\-Studio warning: [V3219](https://pvs-studio.com/en/docs/warnings/v3219/) The 'lineNumber' variable was changed after it was captured in a LINQ method with deferred execution\. The original value will not be used when the method is executed\. FutureMarginBuyingPowerModelTests\.cs 720

The analyzer reports that the `lineNumber` variable passed to a `LINQ` method changes after it is passed\. Indeed, `lineNumber` is initialized and passed to the `LINQ` method, but execution is deferred\. Later, during iteration over the `csv` collection, with each reference to this collection the `lineNumber` value changes\. Then it changes again inside the `foreach` loop\.

In this specific case, the printed `lineNumber` will be twice the expected value due to dual incrementation—in the `LINQ` method and in `foreach`\.

**Snippet 3**

```cpp
 public override void OnSecuritiesChanged(SecurityChanges changes)
{
  if (changes.AddedSecurities.All(s => s.Type != SecurityType.Option))
  {
    return;
  }

  var changeOptions = changes.AddedSecurities
                             .Concat(changes.RemovedSecurities
                             .Where(s => s.Type == SecurityType.Option);

  if (Time != Time.Date)
  {
    throw new RegressionTestException($"Expected options filter to be run 
                                        only at midnight. Actual was {Time}");
  }
}
```

The PVS\-Studio warning: [V3220](https://pvs-studio.com/en/docs/warnings/v3220/) The result of the 'Where' LINQ method with deferred execution is never used The method will not be executed\. AddOptionWithOnMarketOpenOnlyFilterRegressionAlgorithm\.cs 58

The message reports that the `LINQ` result isn't used—the `Where` method employs deferred execution and will never be called\. 

This suggests the missing logic: the developer likely forgot to implement intended functionality here\.

**Snippet 4**

```cpp
var start = DateTime.MinValue;

var symbolMultipliers = LoadSymbolMultipliers();

Parallel.ForEach(files, file =>
{
  try
  {
    Log.Trace("Remote File :" + file);

    var csvFile = Path.Combine(_source.FullName,
                               Path.GetFileNameWithoutExtension(file.Name));

    Log.Trace("Source File :" + csvFile);

    if (!File.Exists(csvFile))
    {
      var csvFileInfo = new FileInfo(csvFile);
      Directory.CreateDirectory(csvFileInfo.DirectoryName);

      Log.Trace("AlgoSeekFuturesConverter.Convert(): Extracting " + file);

      Compression.Extract7ZipArchive(file.FullName, _source.FullName, -1);
      }

    // setting up local processors
    var processors = new Processors();

    var reader = new AlgoSeekFuturesReader(csvFile, 
                                           symbolMultipliers, 
                                           _symbolFilter);
    if (start == DateTime.MinValue)
    {
      start = DateTime.Now;                                     // <=
    }

    ....

  }
});
```

The PVS\-Studio warning: [V3190](https://pvs-studio.com/en/docs/warnings/v3190/) Concurrent modification of the 'start' variable may lead to errors\. AlgoSeekFuturesConverter\.cs 115

The analyzer issued this warning because concurrent modification of the `start` variable inside the parallel `ForEach` method risks a data race\.

When multiple threads modify the same variable simultaneously, results become unpredictable\. This may happen when one thread is reading a variable while another one is changing it\. One can mitigate this by implementing locks\.

### Insidious NRE

**Snippet 5**

```cpp
if (   !ReferenceEquals(result, OptimizationResult.Initial) 
    && string.IsNullOrEmpty(result?.JsonBacktestResult))
{
  _runningParameterSet.Remove(result.ParameterSet); 
  return;
}
....
```

The PVS\-Studio warning: [V3042](https://pvs-studio.com/en/docs/warnings/v3042/) Possible NullReferenceException\. The '?\.' and '\.' operators are used for accessing members of the 'result' object\. EulerSearchOptimizationStrategy\.cs 73

The analyzer detected a possible `NullReferenceException` when accessing the `result` parameter\. The condition is flawed—the `then` block executes precisely when `result` is `null` \(the opposite of what developers intended\)\. Here is the code fix:

```cpp
if (   !ReferenceEquals(result, OptimizationResult.Initial) 
    && !string.IsNullOrEmpty(result?.JsonBacktestResult))
{
  _runningParameterSet.Remove(result.ParameterSet);      // <=
  return;
}
....
```

**Snippet 6**

```cpp
private static void ApplySplitOrDividendToVolatilityModel(IAlgorithm algorithm,
                                                          Security security,
                                                          bool liveMode,
                                   DataNormalizationMode dataNormalizationMode)
{
  if (   security.Type == SecurityType.Equity                             // <=
      && (liveMode || dataNormalizationMode == DataNormalizationMode.Raw))
  {
    security?.VolatilityModel.WarmUp(algorithm.HistoryProvider,           // <=
                                     algorithm.SubscriptionManager, 
                                     security, algorithm.UtcTime,
                                     algorithm.TimeZone, 
                                     liveMode, 
                                     dataNormalizationMode);
  }
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'security' object was used before it was verified against null\. Check lines: 966, 968\. AlgorithmManager\.cs 966

The analyzer detected a scenario that involves using the `security` parameter before checking it for null via `?`, risking to get the `NullReferenceException`\.

If the parameter might be `null`, the authors should consider using `?` for all accesses\. 

**Snippet 7**

```cpp
public void ProcessDelistings(Delistings delistings)
{
  foreach (var delisting in delistings?.Values
                                       .OrderBy(x => !x.Symbol
                                                       .SecurityType
                                                       .IsOption()))
  {    
     ....   
  }
}
```

I suggest you try on the static analyzer role: see if you can find the `NullReferenceException` above and consider fixes\.

<details>
   <summary>The right answer is somewhere around here</summary>

The PVS\-Studio warning: [V3153](https://pvs-studio.com/en/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. BacktestingBrokerage\.cs 527

The issue reappears with the `?` operator—`foreach` doesn't handle `null` collections\. If `delistings` could be `null`, either check it before `foreach` or rewrite as follows:



```cpp
public void ProcessDelistings(Delistings delistings)
{
  foreach (var delisting in delistings?.Values
                                       .OrderBy(x => !x.Symbol
                                                       .SecurityType
                                                       .IsOption())
                            ?? Enumerable.Empty<Delistings>())
  {    
     ....   
  }
}
```

In this case, even if `delistings` is `null`, no exception will follow, and the loop will iterate over an empty collection\.


</details>
### Copy\-paste errors

Let's look at the errors likely arising from copied code where critical details went unmodified\. These bugs severely impact functionality and evade manual detection due to visually similar identifiers\.

Static analysis comes to aid here\. As an example, the PVS\-Studio tool efficiently spots such errors—let's review examples\.

**Snippet 8**

```cpp
public static SortedDictionary<....> Filter<....>(....)
{
  ....
  var breakAfterFailure =
      comparison == BinaryComparison.LessThanOrEqual ||
      comparison == BinaryComparison.LessThanOrEqual;
  ....
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'comparison \=\= BinaryComparison\.LessThanOrEqual' to the left and to the right of the '\|\|' operator\.BinaryComparisonExtensions\.cs 89

The analyzer reports identical expressions on both sides of`||`\. While unclear whether the duplication is erroneous or redundant, this warning is so time\-saving—manual detection of such issues across hundreds of lines is arduous\.

Identical code and analyzer warnings appear elsewhere in the project\. Copy paste strikes again\!

**Snippet 9**

```cpp
public override bool CanSubmitOrder(Security security, 
                                    Order order, 
                                    out BrokerageMessageEvent message)
{
  ....

  if (security.Type != SecurityType.Forex &&
      security.Type != SecurityType.Equity &&
      security.Type != SecurityType.Index &&          // <=
      security.Type != SecurityType.Option &&
      security.Type != SecurityType.Future &&
      security.Type != SecurityType.Cfd &&
      security.Type != SecurityType.Crypto &&
      security.Type != SecurityType.Index)            // <=
  {
    message = new BrokerageMessageEvent(BrokerageMessageType.Warning, 
                                        "NotSupported",
    Messages.DefaultBrokerageModel
            .UnsupportedSecurityType(this, security));
        return false;
    }

    return true;
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'security\.Type \!\= SecurityType\.Index' to the left and to the right of the '&&' operator\. ExanteBrokerageModel\.cs 80

Another oversight—the analyzer highlights two identical subexpressions\. However, it's difficult to say whether this is a significant error\. The second subexpression may be redundant but correct\. However, authors might have intended to check something else with it\.

**Snippet 10**

```cpp
public BacktestResultPacket(BacktestNodePacket job, 
                            BacktestResult results, 
                            DateTime endDate, 
                            DateTime startDate, 
                            decimal progress = 1m) : this()
{
  try
  {
    Progress = Math.Round(progress, 3);
    SessionId = job.SessionId;                   // <=
    PeriodFinish = endDate;
    PeriodStart = startDate;
    CompileId = job.CompileId;       
    Channel = job.Channel;
    BacktestId = job.BacktestId;
    OptimizationId = job.OptimizationId;
    Results = results;
    Name = job.Name;
    UserId = job.UserId;
    ProjectId = job.ProjectId;
    SessionId = job.SessionId;                   // <=
    TradeableDates = job.TradeableDates;
  }
  catch (Exception err) {
    Log.Error(err);
 }
}
```

The PVS\-Studio warning: [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) The 'SessionId' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 177, 166\. BacktestResultPacket\.cs 177

Double assignment of the same value rarely results in an error but may signal misplaced variables\.

### Technical debt fights back

We all enjoy reuniting with old friends—unless that "friend" is a resurrected code bug\.

As mentioned earlier, we previously analyzed this project's potential errors\. We've covered new warnings, now let's revisit older ones that are still relevant today\.

**Snippet 11**

```cpp
public virtual DateTime NextDate(DateTime minDateTime, 
                                 DateTime maxDateTime, 
                                 DayOfWeek? dayOfWeek)
{
    
  ....

  // both are valid dates, so chose one randomly
  if (IsWithinRange(nextDayOfWeek, minDateTime, maxDateTime) &&
        IsWithinRange(previousDayOfWeek, minDateTime, maxDateTime))
  {
    return _random.Next(0, 1) == 0
        ? previousDayOfWeek
        : nextDayOfWeek;
  }

  ....  
}
```

The PVS\-Studio warning: [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression '\_random\.Next\(0, 1\) \=\= 0' is always true\. RandomValueGenerator\.cs 101

This code intends to randomly choose between two values\. But the second value passed to `Next` lies outside the return range\. Here, the method only returns 0\.

Here's a way to fix it:

```cpp
return _random.Next(0, 2) == 0
        ? previousDayOfWeek : nextDayOfWeek;
```

Now the `Next` method will return 0 or 1\.

**Snippet 12**

```cpp
public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  var futureMarginModel = buyingPowerModel as FutureMarginModel;
  if (buyingPowerModel == null)
  {
    throw new Exception($"Invalid buying power model. " +
                        $"Found: {buyingPowerModel.GetType().Name}. " +
                        $"Expected: {nameof(FutureMarginModel)}");  
  }
  ....
}
```

The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'buyingPowerModel'\. BasicTemplateFuturesAlgorithm\.cs 107

The PVS\-Studio warning: [V3019](https://pvs-studio.com/en/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'buyingPowerModel', 'futureMarginModel'\. BasicTemplateFuturesAlgorithm\.cs 105

The analyzer issued two warnings here due to a bizarre condition: a variable is checked for `null` and is dereferenced right after that\.

Most likely, the `as` operator result \(`futureMarginModel` variable\) should be checked for `null` instead\. A corrected version may be as follows:

```cpp
public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  var futureMarginModel = buyingPowerModel as FutureMarginModel;
  if (futureMarginModel == null)
  {
    throw new Exception($"Invalid buying power model. " +
                        $"Found: {buyingPowerModel.GetType().Name}. " +
                        $"Expected: {nameof(FutureMarginModel)}");  
  }
  ....
}
```

Additionally, if `futureMarginModel` is `null`, `buyingPowerModel` might also be `null`, resulting in the `NullReferenceException` during method invocation\. I suggest the following refinement:

```cpp
public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  var futureMarginModel = buyingPowerModel as FutureMarginModel;
  if (futureMarginModel == null)
  {
    string foundType =    buyingPowerModel?.GetType().Name 
                       ?? "the type was not found because the variable is null";
    throw new Exception($"Invalid buying power model. " +
                        $"Found: {foundType}. " +
                        $"Expected: {nameof(FutureMarginModel)}");   
  }
  ....
}
```

**Snippet 13**

```cpp
public class FisherTransform : BarIndicator, IIndicatorWarmUpPeriodProvider
{
  private readonly Minimum _medianMin;
  private readonly Maximum _medianMax;
  ....
  public override bool IsReady => _medianMax.IsReady && _medianMax.IsReady;
  ....
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions '\_medianMax\.IsReady' to the left and to the right of the '&&' operator\. FisherTransform\.cs 72

Identical subexpressions wrap the `&&` operator\. The second one most likely should contain the `medianMin` field\. A corrected version might look as follows:

```cpp
public class FisherTransform : BarIndicator, IIndicatorWarmUpPeriodProvider
{
  private readonly Minimum _medianMin;
  private readonly Maximum _medianMax;
  ....
  public override bool IsReady => _medianMin.IsReady && _medianMax.IsReady;
  ....
}
```

**Snippet 14**

```cpp
if (twoMonthsPriorToContractMonth.Month == 2)
{
  lastBusinessDay = FuturesExpiryUtilityFunctions
                    .NthLastBusinessDay(twoMonthsPriorToContractMonth, 
                                        1, 
                                        holidays);
}
else
{
  lastBusinessDay = FuturesExpiryUtilityFunctions
                    .NthLastBusinessDay(twoMonthsPriorToContractMonth, 
                                        1, 
                                        holidays);
}
```

The PVS\-Studio warning: [V3004](https://pvs-studio.com/en/docs/warnings/v3004/) The 'then' statement is equivalent to the 'else' statement\. FuturesExpiryFunctions\.cs 2380

The analyzer found a case with identical `then` and `else` branches—a classic copy\-paste error where post\-copy logic adjustments were overlooked\.

## To sum up

Considering its scale, the project contains relatively few bugs\. Most appear to stem from oversight rather than complexity\. Notably, bugs from our [previous article](https://pvs-studio.com/en/blog/posts/csharp/0782/) posted 5 years ago remain unpatched today\.

However, one should keep in mind, that error density typically scales with the codebase growth\. The bugs from this and the previous article could have been caught and fixed at the writing code stage using static analysis\.

Request a trial license and try the latest PVS\-Studio version [here](https://pvs-studio.com/en/pvs-studio/try-free/)\.

Take care of your code—and of yourself\!