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
3 changes: 2 additions & 1 deletion src/Neo/Hardfork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public enum Hardfork : byte
{
HF_Aspidochelone,
HF_Basilisk,
HF_Cockatrice
HF_Cockatrice,
HF_Domovoi // from https://github.com/neo-project/neo/pull/3290/files
Copy link
Member

Choose a reason for hiding this comment

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

Remove it, we need to merge #3290 firstly and then rebase this PR onto master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will update it if that one is merged first, otherwise this pr will be the one name the hardfork, no problem at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AnnaShaleva don't introduce dependency here imo

Copy link
Member

Choose a reason for hiding this comment

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

don't introduce dependency here imo

I just don't like the code duplication between PRs. #3290 is ready for merge and should be included into this release as long as this PR.

Copy link
Contributor Author

@Jim8y Jim8y Jun 7, 2024

Choose a reason for hiding this comment

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

don't introduce dependency here imo

I just don't like the code duplication between PRs. #3290 is ready for merge and should be included into this release as long as this PR.

Duplicate is because we need to apply a name for the hf, next time maybe you can create a pr to add a hf before fixing anything. But this is not functional related, we can keep it this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

don't introduce dependency here imo

I just don't like the code duplication between PRs. #3290 is ready for merge and should be included into this release as long as this PR.

imagine someday if there's a urgent bug comes, do you think we should wait for another PR just for applying for a HF_CODE

maybe we should have special HF_CODE for hotfixs

maybe we should not mix the HF_CODE assigning pr into another pr

we need more reasonable branch maintain rules i think

}
}
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
};
}
}
}