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

[Neo Core Bug]fix 3300 #3301

Merged
merged 15 commits into from
Jun 11, 2024
11 changes: 8 additions & 3 deletions src/Neo/SmartContract/ApplicationEngine.Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -407,14 +407,19 @@ protected internal void SendNotification(UInt160 hash, string eventName, Array s
/// </summary>
/// <param name="hash">The hash of the specified contract. It can be set to <see langword="null"/> to get all notifications.</param>
/// <returns>The notifications sent during the execution.</returns>
protected internal NotifyEventArgs[] GetNotifications(UInt160 hash)
protected internal Array GetNotifications(UInt160 hash)
{
IEnumerable<NotifyEventArgs> notifications = Notifications;
if (hash != null) // must filter by scriptHash
notifications = notifications.Where(p => p.ScriptHash == hash);
NotifyEventArgs[] array = notifications.ToArray();
var array = notifications.ToArray();
if (array.Length > Limits.MaxStackSize) throw new InvalidOperationException();
return array;
Array notifyArray = new(ReferenceCounter);
foreach (var notify in array)
{
notifyArray.Add(notify.ToStackItem(ReferenceCounter, this));
}
return notifyArray;
}

/// <summary>
Expand Down
28 changes: 24 additions & 4 deletions src/Neo/SmartContract/NotifyEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,31 @@ public void FromStackItem(StackItem stackItem)
public StackItem ToStackItem(ReferenceCounter referenceCounter)
{
return new Array(referenceCounter)
{
ScriptHash.ToArray(),
EventName,
State
};
}

public StackItem ToStackItem(ReferenceCounter referenceCounter, ApplicationEngine engine)
AnnaShaleva marked this conversation as resolved.
Show resolved Hide resolved
{
if (engine.IsHardforkEnabled(Hardfork.HF_Domovoi))
{
ScriptHash.ToArray(),
EventName,
State
};
return new Array(referenceCounter)
{
ScriptHash.ToArray(),
EventName,
State.OnStack ? State : State.DeepCopy(true)
};
}
Jim8y marked this conversation as resolved.
Show resolved Hide resolved

return new Array(referenceCounter)
{
ScriptHash.ToArray(),
EventName,
State
};
}
}
}
34 changes: 34 additions & 0 deletions tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Neo.Network.P2P.Payloads;
using Neo.SmartContract;
using Neo.VM;
using Neo.VM.Types;

namespace Neo.UnitTests.SmartContract
{
Expand All @@ -27,5 +29,37 @@ public void TestGetScriptContainer()
NotifyEventArgs args = new NotifyEventArgs(container, script_hash, "Test", null);
args.ScriptContainer.Should().Be(container);
}


[TestMethod]
public void TestIssue3300() // https://github.com/neo-project/neo/issues/3300
{
using var engine = ApplicationEngine.Create(TriggerType.Application, null, null, settings: TestProtocolSettings.Default, gas: 1100_00000000);
using (var script = new ScriptBuilder())
{
// Build call script calling disallowed method.
script.Emit(OpCode.NOP);
// Mock executing state to be a contract-based.
engine.LoadScript(script.ToArray());
}

var ns = new Array(engine.ReferenceCounter);
for (var i = 0; i < 500; i++)
{
ns.Add("");
};

var hash = UInt160.Parse("0x179ab5d297fd34ecd48643894242fc3527f42853");
engine.SendNotification(hash, "Test", ns);
// This should have being 0, but we have optimized the vm to not clean the reference counter
// unless it is necessary, so the reference counter will be 1000.
// Same reason why its 1504 instead of 504.
Assert.AreEqual(1000, engine.ReferenceCounter.Count);
// This will make a deepcopy for the notification, along with the 500 state items.
engine.GetNotifications(hash);
// With the fix of issue 3300, the reference counter calculates not only
// the notifaction items, but also the subitems of the notification state.
Assert.AreEqual(1504, engine.ReferenceCounter.Count);
}
}
}