Skip to content

Make plugin ordering culture-invariant and deterministic#9254

Closed
sashaodessa wants to merge 5 commits into
NethermindEth:masterfrom
sashaodessa:master
Closed

Make plugin ordering culture-invariant and deterministic#9254
sashaodessa wants to merge 5 commits into
NethermindEth:masterfrom
sashaodessa:master

Conversation

@sashaodessa

Copy link
Copy Markdown
Contributor
  • Summary: Replace culture-sensitive string operations in PluginLoader.OrderPlugins with culture-invariant, deterministic comparisons.
  • Changes:
    • Replace ToLower()/CompareTo with:
      • StringComparer.OrdinalIgnoreCase for the ordering map
      • string.Compare(..., StringComparison.Ordinal) for tie-break sorting
    • Build orderMap to avoid linear IndexOf lookups.
  • Why: Prevent non-deterministic behavior on non-EN locales (e.g., Turkish “i”) and ensure consistent plugin order across environments.

Comment thread src/Nethermind/Nethermind.Api/Extensions/PluginLoader.cs Outdated
Comment thread src/Nethermind/Nethermind.Api/Extensions/PluginLoader.cs Outdated
sashaodessa and others added 2 commits September 7, 2025 18:55
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
@LukaszRozmej

Copy link
Copy Markdown
Member

Ok I'm now trying to think does it really make that much sense...

  1. Plugin's are never ordered by name - only looked up by it, thus locale shouldn't really matter
  2. There is only a small handful of ordered plugins (literally a few), so not sure if worth paying for a Dictionary - probably doesn't matter much one way or the other.

@sashaodessa

Copy link
Copy Markdown
Contributor Author

Ok I'm now trying to think does it really make that much sense...

  1. Plugin's are never ordered by name - only looked up by it, thus locale shouldn't really matter
  2. There is only a small handful of ordered plugins (literally a few), so not sure if worth paying for a Dictionary - probably doesn't matter much one way or the other.

@LukaszRozmej Thanks for the thoughtful take. The goal here isn’t performance; it’s to make the behavior fully deterministic and culture‑invariant across machines.

  • On (1): Even if we never sort by name, any OrderBy/Sort or key lookup that relies on the default string comparer is culture‑sensitive. On nodes with different locales (e.g., tr‑TR), the i/İ rules change ordering and equality, which risks non‑determinism and flaky tests.
  • On (2): The cost is essentially zero. Using StringComparer.OrdinalIgnoreCase is cheaper than culture‑aware comparisons; with only a few ordered plugins, the overhead is negligible. After construction there are no extra allocations.

Comment on lines +116 to +126
if (!fHas)
{
if (sPos == -1)
if (!sHas)
{
return f.Name.CompareTo(s.Name);
return string.Compare(f.Name, s.Name, StringComparison.OrdinalIgnoreCase);
}

return 1;
}

if (sPos == -1)
if (!sHas)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw you didn't rename the usages of the flags.

@sashaodessa

Copy link
Copy Markdown
Contributor Author

@LukaszRozmej updated

@LukaszRozmej LukaszRozmej self-requested a review September 8, 2025 10:56
@sashaodessa

Copy link
Copy Markdown
Contributor Author

@LukaszRozmej

@LukaszRozmej

Copy link
Copy Markdown
Member

This somewhat conflicts with #9264, which we need to resolve first

@LukaszRozmej LukaszRozmej mentioned this pull request Sep 18, 2025
12 tasks
@LukaszRozmej

Copy link
Copy Markdown
Member

Closed in favor of #9308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants