Skip to content

Native: move hardforks management to native Policy - #4726

Open
cschuchardt88 wants to merge 26 commits into
neo-project:master-n3from
cschuchardt88:fix/hardfork-policy-activation
Open

cschuchardt88 wants to merge 26 commits into
neo-project:master-n3from
cschuchardt88:fix/hardfork-policy-activation

Conversation

@cschuchardt88

@cschuchardt88 cschuchardt88 commented Aug 9, 2026 •

Copy link
Copy Markdown
Member

Close #4580.

@github-actions github-actions Bot added the N3 label Aug 9, 2026
@codecov

codecov Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.48120% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.94%. Comparing base (99618f4) to head (805ca59).

Files with missing lines Patch % Lines
src/Neo/SmartContract/Native/PolicyContract.cs 93.15% 3 Missing and 2 partials ⚠️
src/Neo/Ledger/Blockchain.cs 0.00% 2 Missing ⚠️
src/Neo/Hardfork.cs 94.11% 0 Missing and 1 partial ⚠️
...rc/Neo/SmartContract/ApplicationEngine.Contract.cs 66.66% 0 Missing and 1 partial ⚠️
src/Neo/SmartContract/Native/Notary.cs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master-n3    #4726    +/-   ##
===========================================
  Coverage      83.93%   83.94%            
===========================================
  Files            240      242     +2     
  Lines          17129    17233   +104     
  Branches        2453     2481    +28     
===========================================
+ Hits           14378    14466    +88     
- Misses          1964     1974    +10     
- Partials         787      793     +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cschuchardt88

cschuchardt88 commented Aug 9, 2026 •

Copy link
Copy Markdown
Member Author

Heads up @roman-khimov @AnnaShaleva — this implements #4580 (committee Policy hardfork activation), including the override-safety pieces from the discussion: public-network lock (MainNet magic + setPublicNetwork), private-only HardforkDebugOverrides for neo-express/dev, and HF_Iara as the first post-Huyao Policy-activatable hardfork. Review welcome when you have time.

Add Policy.enableHardfork/getHardfork (HF_Huyao) so post-Huyao hardforks
activate from the next block after a committee-signed call. Wire IsHardforkEnabled,
native init, and contract state to honor on-chain Policy heights; unknown hardfork
ids fail the block so outdated nodes stop until upgraded.
Address neo#4580 comment on override safety: well-known MainNet magic and
on-chain setPublicNetwork seal public chains so post-Huyao activation is
Policy-only. Add HardforkDebugOverrides for neo-express/private nets, HF_Ifrit
as the first Policy-activatable hardfork, config divergence detection, and
tests for misconfiguration failure modes.
Cover private-net enable rejections, public-config issue reporting, HardforkDebugOverrides validation/load paths, and Policy-aware NativeContract init/active helpers after rebasing onto HF_Iara.
@cschuchardt88
cschuchardt88 force-pushed the fix/hardfork-policy-activation branch from b8f5763 to 08f3acf Compare August 9, 2026 23:00
@AnnaShaleva

Copy link
Copy Markdown
Member

@cschuchardt88 the final design of this feature is discussed with @roman-khimov and presented in nspcc-dev/neo-go#4377. So we need C# version to follow this PR. I haven't yet compared your implementation with nspcc-dev/neo-go#4377, will review deeply later.

@cschuchardt88

Copy link
Copy Markdown
Member Author

@AnnaShaleva can you check #4580 (comment) and give me some insights my concerns.

Comment thread src/Neo/SmartContract/Native/NativeContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/NativeContract.cs
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Address Anna's review on neo#4726 / nspcc-dev/neo-go#4377:

- Rename enableHardfork/getHardfork to activateHardfork/getHardforkActivationHeight
- Use raw hardfork names (Iara) and return Null when unset
- Rename HardforkEnabled to HardforkActivationScheduled
- UnknownHardforkException after AssertCommittee so outdated nodes halt
- Drop public-network marker and HardforkDebugOverrides; post-Huyao is Policy-only
- Require a snapshot in NativeContract.IsActive
@cschuchardt88 cschuchardt88 changed the title feat(Policy): enable hardforks via committee transaction (neo#4580) feat(Policy): activate hardforks via committee transaction (neo#4580) Aug 20, 2026
@cschuchardt88

Copy link
Copy Markdown
Member Author

Thanks @AnnaShaleva — this push aligns the C# side with your review and nspcc-dev/neo-go#4377.

Addressed:

  • IsActive now requires a snapshot (no null-passing overload).
  • Dropped Prefix_PublicNetwork / setPublicNetwork / getPublicNetwork — only hardfork heights are stored.
  • Event renamed to HardforkActivationScheduled; hardfork payload is the raw string (Iara), not an int.
  • Removed the #region.
  • getHardfork → getHardforkActivationHeight; enableHardfork → activateHardfork.
  • Hardfork parameter is a string (Iara). Unset/unknown getter returns Null instead of -1.
  • Unknown names throw UnknownHardforkException after AssertCommittee. Blockchain.Persist rethrows it so the node stops instead of FAULTing the tx. Random senders cannot halt the node.
  • Removed HardforkDebugOverrides and the public/private network split. Post-Huyao activation is Policy-only.

Not in this push (same TODO as neo-go #4377): moving Aspidochelone–Huyao activations into Policy and initializing them at Huyao. Happy to do that as a follow-up once the neo-go design for it lands.

Comment thread src/Neo/SmartContract/Native/NativeContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/NativeContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/NativeContract.cs
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread tests/Neo.UnitTests/SmartContract/Native/UT_PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs
Comment thread tests/Neo.UnitTests/SmartContract/Native/UT_NativeContract.cs Outdated
Comment thread tests/Neo.UnitTests/SmartContract/Native/UT_PolicyContract.cs Outdated
Comment thread tests/Neo.UnitTests/SmartContract/Native/UT_PolicyContract.cs
vncoelho and others added 2 commits August 25, 2026 08:54
- Parse Policy hardfork names as the exact raw string only (Iara, not iara/HF_Iara).
- IsHardforkEnabledDelegate now takes settings and snapshot; drop the
  settings-only GetContractState overload.
- Restore the omitted-hardfork-is-disabled comment in IsInitializeBlock.
- activateHardfork uses engine.IsHardforkEnabled only; notify with the raw name.
- Store A-H config heights in Policy on Huyao init; Policy.Activations includes Iara.
- Reject post-Huyao entries in ProtocolSettings.Hardforks.
- Remove DetectHardforkConfigDivergence and redundant Policy tests.
@cschuchardt88

Copy link
Copy Markdown
Member Author

Thanks @AnnaShaleva — this push addresses your 2026-08-25 review.

Policy names and activateHardfork

  • Hardfork argument is the exact raw string only (Iara). iara and HF_Iara are rejected (unknown name after committee).
  • Dropped the extra snapshot Contains check; engine.IsHardforkEnabled(hf) is enough.
  • Notification is [hardfork, activationHeight] using the input name.
  • Error messages use the input name (must be activated via ProtocolSettings configuration, not Policy / already enabled).
  • Removed the instance IsHardforkEnabled(snapshot, hf, index) overload; the static (settings, snapshot, hf, index) form is used everywhere.

NativeContract

  • IsHardforkEnabledDelegate now takes ProtocolSettings and IReadOnlyStore? so the snapshot dependency is explicit. Native cache uses PolicyContract.IsHardforkEnabled as the method group.
  • Removed GetContractState(settings, blockHeight). Remaining overload requires a snapshot.
  • Restored the omitted-hardfork-is-disabled comment in IsInitializeBlock.
  • IsActive always goes through Policy's combined checker.

Huyao init / Activations

  • InitializeAsync(HF_Huyao) copies config-managed A–H heights into Policy storage.
  • Policy.Activations is [null, HF_Iara] (same pattern as Notary), so Iara is a Policy initialize height.

ProtocolSettings

  • CheckingHardfork rejects post-Huyao entries (HF_Iara in config throws).
  • Removed DetectHardforkConfigDivergence (tests-only).
  • Removed Hardforks.TryParse (case-insensitive / HF_ prefix); replaced with TryParseExact.

Tests

  • Dropped the Policy ABI/manifest test (ContractManagement already covers it).
  • Dropped the local-config Iara test; added Load-rejects-Iara and case-sensitive name tests.
  • Huyao init test asserts A–H heights are stored.
  • IsInitializeBlock at a Policy-stored Iara height is true via Activations.

78 related unit tests passed (UT_PolicyContract, UT_NativeContract, UT_ProtocolSettings).

After Huyao initialize copies Aspidochelone-Huyao heights into Policy,
TryGetActivationHeight prefers those stored heights so IsActive is
Policy-backed (neo-go#4377). Config is used only until Huyao is enabled.
@cschuchardt88

Copy link
Copy Markdown
Member Author

Completed the remaining neo-go#4377 / Anna TODO on this PR: use Policy as the runtime source of truth for Aspidochelone–Huyao once Huyao is enabled.

Huyao InitializeAsync already copied A–H heights into Policy storage. TryGetActivationHeight now:

  • takes the block index
  • for post-Huyao forks: Policy only
  • for A–H: Policy after Huyao is enabled in config (the copy exists); ProtocolSettings.Hardforks until then

So IsActive / IsInitializeBlock stop consulting local config for A–H after Huyao, matching Roman’s “initialize A–H at Huyao, then keep only Policy in IsActive.” getHardforkActivationHeight("Aspidochelone") also returns the stored height.

Tests: Check_AfterHuyao_ConfigManagedHeightsComeFromPolicy (Policy wins over a mismatched local Faun height), plus Huyao init / initialize-block coverage. 79 related tests passed.

Not in this PR: the same A–H copy inside nspcc-dev/neo-go#4377, and committee multi-sig packaging for activateHardfork txs.

Cover getHardforkActivationHeight after activate, missing Policy
height, activate without a persisting block, UnknownHardforkException
wrapping, and Hardforks.TryParseExact/GetName.
Comment thread src/Neo/SmartContract/Native/NativeContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/PolicyContract.cs Outdated
Comment thread src/Neo/SmartContract/Native/NativeContract.cs Outdated
Comment thread src/Neo/Hardfork.cs
Comment thread src/Neo/ProtocolSettings.cs Outdated
Comment thread tests/Neo.UnitTests/SmartContract/Native/UT_PolicyContract.cs Outdated
@AnnaShaleva
AnnaShaleva force-pushed the fix/hardfork-policy-activation branch from 04df42d to b69df3a Compare September 15, 2026 11:50
@AnnaShaleva AnnaShaleva changed the title feat(Policy): activate hardforks via committee transaction (neo#4580) Policy: move hardforks management to native Policy Sep 15, 2026
@vncoelho

Copy link
Copy Markdown
Member

Yes, @AnnaShaleva
Thanks for this update.

But my main concern is regard the previous conducted tests . If anyone has already tested...anyway.

The PR looks good. I am finishing some basic testing.+

Comment thread src/Neo/Ledger/Blockchain.cs
@AnnaShaleva
AnnaShaleva requested a review from shargon September 28, 2026 09:09
if (engine.SnapshotCache.Contains(key))
throw new InvalidOperationException($"Hardfork {hardfork} is already scheduled.");

uint activationHeight = checked(engine.PersistingBlock.Index + 1);

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.

Maybe we should send the num of blocks after activation, to have margin between sign, relay, and fork

@AnnaShaleva AnnaShaleva Sep 30, 2026 •

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.

Can be done, but why do we need it? We don't have a mechanism of scheduled hardfork deactivation anyway.

send the num of blocks

Do you want it to be a parameter of ActivateHardfork method?

@shargon shargon Oct 1, 2026 •

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.

Preparation, you can publish that fork will be in specific height, like this is not possible to know when will be active

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To me it's pretty useless, I'd rather have it activated once transaction gets in, it's just easier to handle. We don't know when block X will happen anyway.

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.

Contracts can be prepared, with this new logic you are no only changing how hardfork will be activated, you are changing how and when the users will know it

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.

Maybe we should send the num of blocks after activation, to have margin between sign, relay, and fork

Or we set the number of blocks after agreement when required number of signatures is reached (but remember that this will be prune to interpretation of txs and should be ensured by onchain TXs only).

Or, at least, we need a fixed height upon agreement, if not reached agreement until that we try again. That is the best model to me. Fixed height and tentative until it.

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.

Preparation, you can publish that fork will be in specific height, like this is not possible to know when will be active

My idea is the same, @shargon

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.

To me it's pretty useless, I'd rather have it activated once transaction gets in, it's just easier to handle. We don't know when block X will happen anyway.

I do not think so, to much uncertainties, too much prone to manipulation. Fixed height and fixed rule with a Preparation.

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.

Contracts can be prepared, with this new logic you are no only changing how hardfork will be activated, you are changing how and when the users will know it

I am fully aligned with you, @shargon

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.

Implemented, @shargon let's check the updated version. We also added an ability to reschedule the already scheduled hardfork. It's useful in case if hardfork is already scheduled but some bug is discovered and a fix should be applied prior to hardfork activation.

return false;
}

if (settings.IsHardforkEnabled(ProtocolSettings.LastConfigManagedHardfork, currentIndex)

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.

Why check of is enabled? You want to know the activation height

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.

Because starting from LastConfigManagedHardfork we store height in the Policy's storage. So if LastConfigManagedHardfork is enabled then we should refer to the Policy instead of config.

Comment thread src/Neo/Hardfork.cs Outdated
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva
AnnaShaleva requested a review from shargon September 30, 2026 17:35
Comment thread src/Neo/Hardfork.cs Outdated
return false;

foreach (Hardfork value in Enum.GetValues<Hardfork>())
if (Enum.TryParse(typeof(Hardfork), "HF_" + name, false, out var fork) && Enum.GetNames<Hardfork>().Any(n => n.Equals("HF_" + name, StringComparison.Ordinal)))

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.

Is not enough with tryParse?

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.

We need to check that the resulting enum is defined. But you're right, we add HF_ prefix to name, so any non-valid name will fail parsing.

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.

Fixed.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva

Copy link
Copy Markdown
Member

Updated to the latest master.

[Flags] lets Enum.TryParse accept comma-separated aliases that equal a
named value. TryParseExact now requires the canonical HF_ name, so
activateHardfork only accepts exact names such as Iara.

The macOS Test job failed
UT_TaskManager.InvalidBlock_AbortsThePeerThatSuppliedIt because
peer.Send returned before the mailbox recorded the hash. Drive the
tests that assert ReceivedBlockHashes immediately through
TestActorRef.Receive.

Ref. neo-project#4726
@cschuchardt88

Copy link
Copy Markdown
Member Author

Pushed 72039f2c. Ubuntu, Windows, and macOS Test are green.

Hardfork name parsing. Hardfork is [Flags], so Enum.TryParse accepts comma-separated aliases that collapse onto a named value (Basilisk, HF_Cockatrice equals HF_Domovoi). Hardforks.TryParseExact now requires the canonical HF_ name. activateHardfork still takes the exact raw string (Iara). iara, HF_Iara, and flag aliases are rejected. Covered in Check_Hardforks_TryParseExact_AndGetName.

macOS TaskManager failure. UT_TaskManager.InvalidBlock_AbortsThePeerThatSuppliedIt failed on macos-latest because peer.Send returned before the actor recorded the block hash. That test and the siblings that assert ReceivedBlockHashes immediately now use TestActorRef.Receive, so the actor runs on the test thread. No production TaskManager change.

cschuchardt88 and others added 2 commits October 5, 2026 23:36
activateHardfork and getHardforkActivationHeight now take ByteArray
(0x12) instead of String (0x13) so the Huyao Policy manifest matches
nspcc-dev/neo-go#4377. Names are still UTF-8 canonical HF names parsed
by Hardforks.TryParseExact.

Ref. neo-project#4726
@AnnaShaleva

Copy link
Copy Markdown
Member

640f1f6 is reverted, we need a fully-qualified String here. NeoGo implementation is out-of-date and will be aligned with the C# core once this PR is finalized.

Ref.
neo-project#4726 (comment).

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva

Copy link
Copy Markdown
Member

@neo-project/core let's review the updated API one more time.

}

[TestMethod]
public void Load_IgnoresHardforkDebugOverrides_AndDoesNotEnableIara()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This test is no longer need. It will always fail.

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.

Agree, removed.

@AnnaShaleva

Copy link
Copy Markdown
Member

This PR slightly depends on #4764, so let's wait for the #4764 to settle and I'll update it afterwards.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trigger hardforks via committee-signed transactions

8 participants