Skip to content

Automatically set Id for native contracts#2210

Merged
erikzhang merged 5 commits intomasterfrom
native/auto-id
Jan 10, 2021
Merged

Automatically set Id for native contracts#2210
erikzhang merged 5 commits intomasterfrom
native/auto-id

Conversation

@erikzhang
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Good change


public static IReadOnlyCollection<NativeContract> Contracts { get; } = contractsList;
#region Named Native Contracts
public static ContractManagement ContractManagement { get; } = new ContractManagement();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we can init this contracts inside the static constructor in order to avoid that the compiler optimizer play with us.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Usually only partial classes have non-deterministic problems.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that it's safer because this part is uncontrolled by us.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But I don't want to modify every native contract each time we create a new one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe if we use this code, we can avoid it?

[MethodImpl(MethodImplOptions.NoOptimization)]
static NativeContract() { }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It won't be any problem without this code.

shargon
shargon previously approved these changes Jan 10, 2021
@erikzhang erikzhang merged commit dfb56a2 into master Jan 10, 2021
@erikzhang erikzhang deleted the native/auto-id branch January 10, 2021 11:37
cloud8little pushed a commit to cloud8little/neo that referenced this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants