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.

Sign up for GitHub

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

Jump to bottom

Avoid re-init properties #902

Closed
wants to merge 4 commits into from
Closed

Avoid re-init properties #902

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e429386
Avoid re-init properties
shargon May 7, 2024
8a91fe8
Merge branch 'master' into core-optimizer-init
shargon May 8, 2024
15aca16
Merge branch 'master' into core-optimizer-init
Jim8y May 10, 2024
6995825
Merge branch 'master' into core-optimizer-init
Jim8y May 10, 2024
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
4 changes: 2 additions & 2 deletions src/ApplicationLogs/Store/Models/ApplicationEngineLogModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace Neo.Plugins.Store.Models
{
public class ApplicationEngineLogModel
{
public UInt160 ScriptHash { get; private init; } = new();
public string Message { get; private init; } = string.Empty;
public required UInt160 ScriptHash { get; init; }
public required string Message { get; init; }

public static ApplicationEngineLogModel Create(EngineLogState logEventState) =>
new()
Expand Down
12 changes: 6 additions & 6 deletions src/ApplicationLogs/Store/Models/BlockchainEventModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,27 @@ namespace ApplicationLogs.Store.Models
{
public class BlockchainEventModel
{
public UInt160 ScriptHash { get; private init; } = new();
public string EventName { get; private init; } = string.Empty;
public StackItem[] State { get; private init; } = [];
public required UInt160 ScriptHash { get; init; }
public required string EventName { get; init; }
public required StackItem[] State { get; init; }

public static BlockchainEventModel Create(UInt160 scriptHash, string eventName, StackItem[] state) =>
public static BlockchainEventModel Create(UInt160 scriptHash, string eventName, params StackItem[] state) =>
new()
{
ScriptHash = scriptHash,
EventName = eventName ?? string.Empty,
State = state,
};

public static BlockchainEventModel Create(NotifyLogState notifyLogState, StackItem[] state) =>
public static BlockchainEventModel Create(NotifyLogState notifyLogState, params StackItem[] state) =>
new()
{
ScriptHash = notifyLogState.ScriptHash,
EventName = notifyLogState.EventName,
State = state,
};

public static BlockchainEventModel Create(ContractLogState contractLogState, StackItem[] state) =>
public static BlockchainEventModel Create(ContractLogState contractLogState, params StackItem[] state) =>
new()
{
ScriptHash = contractLogState.ScriptHash,
Expand Down
18 changes: 10 additions & 8 deletions src/ApplicationLogs/Store/Models/BlockchainExecutionModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,24 @@ namespace ApplicationLogs.Store.Models
{
public class BlockchainExecutionModel
{
public TriggerType Trigger { get; private init; } = TriggerType.All;
public VMState VmState { get; private init; } = VMState.NONE;
public string Exception { get; private init; } = string.Empty;
public long GasConsumed { get; private init; } = 0L;
public StackItem[] Stack { get; private init; } = [];
public BlockchainEventModel[] Notifications { get; set; } = [];
public ApplicationEngineLogModel[] Logs { get; set; } = [];
public required TriggerType Trigger { get; init; }
public required VMState VmState { get; init; }
public required string Exception { get; init; }
public required long GasConsumed { get; init; }
public required StackItem[] Stack { get; init; }
public required BlockchainEventModel[] Notifications { get; set; }
public required ApplicationEngineLogModel[] Logs { get; set; }

public static BlockchainExecutionModel Create(TriggerType trigger, ExecutionLogState executionLogState, StackItem[] stack) =>
public static BlockchainExecutionModel Create(TriggerType trigger, ExecutionLogState executionLogState, params StackItem[] stack) =>
new()
{
Trigger = trigger,
VmState = executionLogState.VmState,
Exception = executionLogState.Exception ?? string.Empty,
GasConsumed = executionLogState.GasConsumed,
Stack = stack,
Notifications = [],
Logs = []
};
}
}
10 changes: 5 additions & 5 deletions src/ApplicationLogs/Store/States/BlockLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ namespace ApplicationLogs.Store.States
{
public class BlockLogState : ISerializable, IEquatable<BlockLogState>
{
public Guid[] NotifyLogIds { get; private set; } = [];
public Guid[] NotifyLogIds { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

like this? Kinda ugly:

public virtual void Deserialize(ref MemoryReader reader)
{
    // It should be safe because it filled from a block's notifications.
    var aLen = reader.ReadUInt32();
    var notifyLogIds = new Guid[aLen];
    for (var i = 0; i < aLen; i++)
        notifyLogIds[i] = new Guid(reader.ReadVarMemory().Span);

    // Create a new instance of BlockLogState using the Create factory method
    var blockLogState = Create(notifyLogIds);

    // Assign the created instance to the current object
    NotifyLogIds = blockLogState.NotifyLogIds;
}


public static BlockLogState Create(Guid[] notifyLogIds) =>
public static BlockLogState Create(params Guid[] notifyLogIds) =>
new()
{
NotifyLogIds = notifyLogIds,
Expand All @@ -33,16 +33,16 @@ public static BlockLogState Create(Guid[] notifyLogIds) =>
public virtual void Deserialize(ref MemoryReader reader)
{
// It should be safe because it filled from a block's notifications.
uint aLen = reader.ReadUInt32();
var aLen = reader.ReadUInt32();
NotifyLogIds = new Guid[aLen];
for (int i = 0; i < aLen; i++)
for (var i = 0; i < aLen; i++)
NotifyLogIds[i] = new Guid(reader.ReadVarMemory().Span);
}

public virtual void Serialize(BinaryWriter writer)
{
writer.Write((uint)NotifyLogIds.Length);
for (int i = 0; i < NotifyLogIds.Length; i++)
for (var i = 0; i < NotifyLogIds.Length; i++)
writer.WriteVarBytes(NotifyLogIds[i].ToByteArray());
}

Expand Down
6 changes: 3 additions & 3 deletions src/ApplicationLogs/Store/States/ContractLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ namespace ApplicationLogs.Store.States
{
public class ContractLogState : NotifyLogState, IEquatable<ContractLogState>
{
public UInt256 TransactionHash { get; private set; } = new();
public TriggerType Trigger { get; private set; } = TriggerType.All;
public UInt256 TransactionHash { get; private set; }
public TriggerType Trigger { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.


public static ContractLogState Create(Blockchain.ApplicationExecuted applicationExecuted, NotifyEventArgs notifyEventArgs, Guid[] stackItemIds) =>
public static ContractLogState Create(Blockchain.ApplicationExecuted applicationExecuted, NotifyEventArgs notifyEventArgs, params Guid[] stackItemIds) =>
new()
{
TransactionHash = applicationExecuted.Transaction?.Hash ?? new(),
Expand Down
4 changes: 2 additions & 2 deletions src/ApplicationLogs/Store/States/EngineLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace Neo.Plugins.Store.States
{
public class EngineLogState : ISerializable, IEquatable<EngineLogState>
{
public UInt160 ScriptHash { get; private set; } = new();
public string Message { get; private set; } = string.Empty;
public UInt160 ScriptHash { get; private set; }
public string Message { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.


public static EngineLogState Create(UInt160 scriptHash, string message) =>
new()
Expand Down
14 changes: 7 additions & 7 deletions src/ApplicationLogs/Store/States/ExecutionLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ namespace ApplicationLogs.Store.States
{
public class ExecutionLogState : ISerializable, IEquatable<ExecutionLogState>
{
public VMState VmState { get; private set; } = VMState.NONE;
public string Exception { get; private set; } = string.Empty;
public long GasConsumed { get; private set; } = 0L;
public Guid[] StackItemIds { get; private set; } = [];
public VMState VmState { get; private set; }
public string Exception { get; private set; }
public long GasConsumed { get; private set; }
public Guid[] StackItemIds { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.


public static ExecutionLogState Create(Blockchain.ApplicationExecuted appExecution, Guid[] stackItemIds) =>
public static ExecutionLogState Create(Blockchain.ApplicationExecuted appExecution, params Guid[] stackItemIds) =>
new()
{
VmState = appExecution.VMState,
Expand All @@ -47,9 +47,9 @@ public void Deserialize(ref MemoryReader reader)
GasConsumed = reader.ReadInt64();

// It should be safe because it filled from a transaction's stack.
uint aLen = reader.ReadUInt32();
var aLen = reader.ReadUInt32();
StackItemIds = new Guid[aLen];
for (int i = 0; i < aLen; i++)
for (var i = 0; i < aLen; i++)
StackItemIds[i] = new Guid(reader.ReadVarMemory().Span);
}

Expand Down
10 changes: 5 additions & 5 deletions src/ApplicationLogs/Store/States/NotifyLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ namespace ApplicationLogs.Store.States
{
public class NotifyLogState : ISerializable, IEquatable<NotifyLogState>
{
public UInt160 ScriptHash { get; protected set; } = new();
public string EventName { get; protected set; } = string.Empty;
public Guid[] StackItemIds { get; protected set; } = [];
public UInt160 ScriptHash { get; protected set; }
public string EventName { get; protected set; }
public Guid[] StackItemIds { get; protected set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.


public static NotifyLogState Create(NotifyEventArgs notifyItem, Guid[] stackItemsIds) =>
public static NotifyLogState Create(NotifyEventArgs notifyItem, params Guid[] stackItemsIds) =>
new()
{
ScriptHash = notifyItem.ScriptHash,
Expand All @@ -42,7 +42,7 @@ public virtual void Deserialize(ref MemoryReader reader)
EventName = reader.ReadVarString();

// It should be safe because it filled from a transaction's notifications.
uint aLen = reader.ReadUInt32();
var aLen = reader.ReadUInt32();
StackItemIds = new Guid[aLen];
for (var i = 0; i < aLen; i++)
StackItemIds[i] = new Guid(reader.ReadVarMemory().Span);
Expand Down
10 changes: 5 additions & 5 deletions src/ApplicationLogs/Store/States/TransactionEngineLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ namespace Neo.Plugins.Store.States
{
public class TransactionEngineLogState : ISerializable, IEquatable<TransactionEngineLogState>
{
public Guid[] LogIds { get; private set; } = Array.Empty<Guid>();
public Guid[] LogIds { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then set can't be private

Copy link
Member

@cschuchardt88 cschuchardt88 May 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

They have Create method and that should be used. Plus you still run into the same problem with null. So you need to add or remove changes on these files. I marked in review.


public static TransactionEngineLogState Create(Guid[] logIds) =>
public static TransactionEngineLogState Create(params Guid[] logIds) =>
new()
{
LogIds = logIds,
Expand All @@ -32,16 +32,16 @@ public static TransactionEngineLogState Create(Guid[] logIds) =>
public virtual void Deserialize(ref MemoryReader reader)
{
// It should be safe because it filled from a transaction's logs.
uint aLen = reader.ReadUInt32();
var aLen = reader.ReadUInt32();
LogIds = new Guid[aLen];
for (int i = 0; i < aLen; i++)
for (var i = 0; i < aLen; i++)
LogIds[i] = new Guid(reader.ReadVarMemory().Span);
}

public virtual void Serialize(BinaryWriter writer)
{
writer.Write((uint)LogIds.Length);
for (int i = 0; i < LogIds.Length; i++)
for (var i = 0; i < LogIds.Length; i++)
writer.WriteVarBytes(LogIds[i].ToByteArray());
}

Expand Down
10 changes: 5 additions & 5 deletions src/ApplicationLogs/Store/States/TransactionLogState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ namespace ApplicationLogs.Store.States
{
public class TransactionLogState : ISerializable, IEquatable<TransactionLogState>
{
public Guid[] NotifyLogIds { get; private set; } = Array.Empty<Guid>();
public Guid[] NotifyLogIds { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

We should make this required as well.


public static TransactionLogState Create(Guid[] notifyLogIds) =>
public static TransactionLogState Create(params Guid[] notifyLogIds) =>
new()
{
NotifyLogIds = notifyLogIds,
Expand All @@ -33,16 +33,16 @@ public static TransactionLogState Create(Guid[] notifyLogIds) =>
public virtual void Deserialize(ref MemoryReader reader)
{
// It should be safe because it filled from a transaction's notifications.
uint aLen = reader.ReadUInt32();
var aLen = reader.ReadUInt32();
Copy link
Member

Choose a reason for hiding this comment

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

Is this related?

Copy link
Member

Choose a reason for hiding this comment

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

No, it's just using dynamic type variable now; instead of being bound to a type specific variable.

NotifyLogIds = new Guid[aLen];
for (int i = 0; i < aLen; i++)
for (var i = 0; i < aLen; i++)
NotifyLogIds[i] = new Guid(reader.ReadVarMemory().Span);
}

public virtual void Serialize(BinaryWriter writer)
{
writer.Write((uint)NotifyLogIds.Length);
for (int i = 0; i < NotifyLogIds.Length; i++)
for (var i = 0; i < NotifyLogIds.Length; i++)
writer.WriteVarBytes(NotifyLogIds[i].ToByteArray());
}

Expand Down
Loading