Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SmartContract: restrict the number of allowed notifications #3548

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Neo.CLI/config.fs.mainnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
"HF_Aspidochelone": 3000000,
"HF_Basilisk": 4500000,
"HF_Cockatrice": 5800000,
"HF_Domovoi": 5800000
"HF_Domovoi": 5800000,
"HF_Echidna": 5800001
},
"StandbyCommittee": [
"026fa34ec057d74c2fdf1a18e336d0bd597ea401a0b2ad57340d5c220d09f44086",
Expand Down
3 changes: 2 additions & 1 deletion src/Neo.CLI/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"HF_Aspidochelone": 1730000,
"HF_Basilisk": 4120000,
"HF_Cockatrice": 5450000,
"HF_Domovoi": 5570000
"HF_Domovoi": 5570000,
"HF_Echidna": 5570001
},
"InitialGasDistribution": 5200000000000000,
"ValidatorsCount": 7,
Expand Down
3 changes: 2 additions & 1 deletion src/Neo.CLI/config.mainnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"HF_Aspidochelone": 1730000,
"HF_Basilisk": 4120000,
"HF_Cockatrice": 5450000,
"HF_Domovoi": 5570000
"HF_Domovoi": 5570000,
"HF_Echidna": 5570001
},
"InitialGasDistribution": 5200000000000000,
"ValidatorsCount": 7,
Expand Down
3 changes: 2 additions & 1 deletion src/Neo.CLI/config.testnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"HF_Aspidochelone": 210000,
"HF_Basilisk": 2680000,
"HF_Cockatrice": 3967000,
"HF_Domovoi": 4144000
"HF_Domovoi": 4144000,
"HF_Echidna": 4144001
},
"InitialGasDistribution": 5200000000000000,
"ValidatorsCount": 7,
Expand Down
3 changes: 2 additions & 1 deletion src/Neo/Hardfork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public enum Hardfork : byte
HF_Aspidochelone,
HF_Basilisk,
HF_Cockatrice,
HF_Domovoi
HF_Domovoi,
HF_Echidna
}
}
1 change: 1 addition & 0 deletions src/Neo/Neo.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="8.0.0" />
<PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.2" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.0" />
<PackageReference Include="Microsoft.IdentityModel.Tokens" Version="8.0.1" />
</ItemGroup>

<ItemGroup>
Expand Down
14 changes: 13 additions & 1 deletion src/Neo/SmartContract/ApplicationEngine.Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ partial class ApplicationEngine
/// </summary>
public const int MaxNotificationSize = 1024;

/// <summary>
/// The maximum number of notifications per application execution.
/// </summary>
public const int MaxNotificationCount = 512;
Copy link
Member

Choose a reason for hiding this comment

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

Use ExecutionEngineLimits Class engine.Limits.MaxNotificationCount

Copy link
Member

Choose a reason for hiding this comment

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

Notifications is a syscall, outside from VM

Copy link
Member

Choose a reason for hiding this comment

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

Its still an Engine Limitation, just cause the VM is splitted into two libraries shouldn't make it be to separate things.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have MaxNotificationSize and MaxEventName declared in the same class as MaxNotificationCount, these three constants must be kept together.


private uint random_times = 0;

/// <summary>
Expand Down Expand Up @@ -395,8 +400,15 @@ protected internal void RuntimeNotifyV1(byte[] eventName, Array state)
protected internal void SendNotification(UInt160 hash, string eventName, Array state)
{
NotifyEventArgs notification = new(ScriptContainer, hash, eventName, (Array)state.DeepCopy(asImmutable: true));
Notify?.Invoke(this, notification);
notifications ??= new List<NotifyEventArgs>();
// Restrict the number of notifications for Application executions. Do not check
shargon marked this conversation as resolved.
Show resolved Hide resolved
// persisting triggers to avoid native persist failure. Do not check verification
// trigger since verification context is loaded with ReadOnly flag.
if (IsHardforkEnabled(Hardfork.HF_Echidna) && Trigger == TriggerType.Application && notifications.Count >= MaxNotificationCount)
shargon marked this conversation as resolved.
Show resolved Hide resolved
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
throw new InvalidOperationException($"Maximum number of notifications `{MaxNotificationCount}` is reached.");
}
Notify?.Invoke(this, notification);
notifications.Add(notification);
CurrentContext.GetState<ExecutionContextState>().NotificationCount++;
}
Expand Down
24 changes: 22 additions & 2 deletions src/Neo/SmartContract/Native/RoleManagement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,16 @@
{
[ContractEvent(0, name: "Designation",
"Role", ContractParameterType.Integer,
"BlockIndex", ContractParameterType.Integer)]
"BlockIndex", ContractParameterType.Integer,
Hardfork.HF_Echidna)]

[ContractEvent(Hardfork.HF_Echidna, 0, name: "Designation",
"Role", ContractParameterType.Integer,
"BlockIndex", ContractParameterType.Integer,
"Old", ContractParameterType.Array,
"New", ContractParameterType.Array
)]

internal RoleManagement() : base() { }

/// <summary>
Expand Down Expand Up @@ -69,7 +78,18 @@
list.AddRange(nodes);
list.Sort();
engine.SnapshotCache.Add(key, new StorageItem(list));
engine.SendNotification(Hash, "Designation", new VM.Types.Array(engine.ReferenceCounter, new StackItem[] { (int)role, engine.PersistingBlock.Index }));

if (engine.IsHardforkEnabled(Hardfork.HF_Echidna))
{
var oldNodes = new VM.Types.Array(engine.ReferenceCounter, GetDesignatedByRole(engine.Snapshot, role, index - 1).Select(u => (ByteString)u.EncodePoint(true)));

Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

'ApplicationEngine.Snapshot' is obsolete: 'This property is deprecated. Use SnapshotCache instead.'

Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

'ApplicationEngine.Snapshot' is obsolete: 'This property is deprecated. Use SnapshotCache instead.'

Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

'ApplicationEngine.Snapshot' is obsolete: 'This property is deprecated. Use SnapshotCache instead.'

Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

'ApplicationEngine.Snapshot' is obsolete: 'This property is deprecated. Use SnapshotCache instead.'

Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

'ApplicationEngine.Snapshot' is obsolete: 'This property is deprecated. Use SnapshotCache instead.'
var newNodes = new VM.Types.Array(engine.ReferenceCounter, nodes.Select(u => (ByteString)u.EncodePoint(true)));

engine.SendNotification(Hash, "Designation", new VM.Types.Array(engine.ReferenceCounter, [(int)role, engine.PersistingBlock.Index, oldNodes, newNodes]));
}
else
{
engine.SendNotification(Hash, "Designation", new VM.Types.Array(engine.ReferenceCounter, [(int)role, engine.PersistingBlock.Index]));
}
}

private class NodeList : InteroperableList<ECPoint>
Expand Down
23 changes: 23 additions & 0 deletions src/Neo/SmartContract/Native/StdLib.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#pragma warning disable IDE0051

using Microsoft.IdentityModel.Tokens;
using Neo.Cryptography;
using Neo.Json;
using Neo.VM.Types;
Expand Down Expand Up @@ -131,6 +132,28 @@ public static byte[] Base64Decode([MaxLength(MaxInputLength)] string s)
return Convert.FromBase64String(s);
}

/// <summary>
/// Encodes a byte array into a base64Url string.
/// </summary>
/// <param name="data">The base64Url to be encoded.</param>
/// <returns>The encoded base64Url string.</returns>
[ContractMethod(Hardfork.HF_Echidna, CpuFee = 1 << 5)]
public static string Base64UrlEncode([MaxLength(MaxInputLength)] string data)
{
return Base64UrlEncoder.Encode(data);
}

/// <summary>
/// Decodes a byte array from a base64Url string.
/// </summary>
/// <param name="s">The base64Url string.</param>
/// <returns>The decoded base64Url string.</returns>
[ContractMethod(Hardfork.HF_Echidna, CpuFee = 1 << 5)]
public static string Base64UrlDecode([MaxLength(MaxInputLength)] string s)
{
return Base64UrlEncoder.Decode(s);
}

/// <summary>
/// Encodes a byte array into a base58 <see cref="string"/>.
/// </summary>
Expand Down
20 changes: 19 additions & 1 deletion tests/Neo.UnitTests/SmartContract/Native/UT_NativeContract.cs

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions tests/Neo.UnitTests/SmartContract/Native/UT_StdLib.cs
Original file line number Diff line number Diff line change
Expand Up @@ -406,5 +406,25 @@ public void TestRuntime_Deserialize()
Assert.AreEqual(engine.ResultStack.Pop<Integer>().GetInteger(), 100);
Assert.AreEqual(engine.ResultStack.Pop<ByteString>().GetString(), "test");
}

[TestMethod]
public void TestBase64Url()
{
var snapshotCache = TestBlockchain.GetTestSnapshotCache();
using (var script = new ScriptBuilder())
{
// Test encoding
script.EmitDynamicCall(NativeContract.StdLib.Hash, "base64UrlEncode", "[email protected]&Issuer=https://example.com");
script.EmitDynamicCall(NativeContract.StdLib.Hash, "base64UrlDecode", "U3ViamVjdD10ZXN0QGV4YW1wbGUuY29tJklzc3Vlcj1odHRwczovL2V4YW1wbGUuY29t");

using var engine = ApplicationEngine.Create(TriggerType.Application, null, snapshotCache, settings: TestBlockchain.TheNeoSystem.Settings);
engine.LoadScript(script.ToArray());

Assert.AreEqual(engine.Execute(), VMState.HALT);
Assert.AreEqual(2, engine.ResultStack.Count);
Assert.AreEqual("[email protected]&Issuer=https://example.com", engine.ResultStack.Pop<ByteString>());
Assert.AreEqual("U3ViamVjdD10ZXN0QGV4YW1wbGUuY29tJklzc3Vlcj1odHRwczovL2V4YW1wbGUuY29t", engine.ResultStack.Pop<ByteString>().GetString());
}
}
}
}
Loading