-
Notifications
You must be signed in to change notification settings - Fork 1k
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
base: HF_Echidna
Are you sure you want to change the base?
Changes from 4 commits
b31d95e
0c72e57
ee21427
4a6f36a
6fff362
4814339
9a657a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,10 +78,10 @@ | |
list.AddRange(nodes); | ||
list.Sort(); | ||
engine.SnapshotCache.Add(key, new StorageItem(list)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Formatting job is failing without this change. |
||
|
||
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 GitHub Actions / Test-Everything
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test-Everything
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test-Everything
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test-Everything
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test (ubuntu-latest)
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test (macos-latest)
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test (macos-latest)
Check warning on line 84 in src/Neo/SmartContract/Native/RoleManagement.cs GitHub Actions / Test (macos-latest)
|
||
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])); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
ExecutionEngineLimits
Classengine.Limits.MaxNotificationCount
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have
MaxNotificationSize
andMaxEventName
declared in the same class asMaxNotificationCount
, these three constants must be kept together.