Skip to content

Conversation

@erikzhang
Copy link
Member

Description

Enable the nullable option for the Neo project.

Type of change

  • Style (the change is only a code style for better maintenance or standard purpose)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • Unit Testing
  • Run Application
  • Local Computer Tests
  • No Testing

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Jim8y
Jim8y previously approved these changes Nov 8, 2025
@Wi1l-B0t Wi1l-B0t assigned Wi1l-B0t and unassigned Wi1l-B0t Nov 8, 2025
@Wi1l-B0t
Copy link
Contributor

Wi1l-B0t commented Nov 8, 2025

Most of the modifications only replace the original impl with !, ?, required, record.

@shargon
Copy link
Member

shargon commented Nov 8, 2025

Give me some days to review it

/// The script of the contract.
/// </summary>
public byte[] Script;
public byte[] Script { get => field ??= []; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Here the logic it's different, we set in the script

Suggested change
public byte[] Script { get => field ??= []; set; }
public byte[] Script { get => field ?? []; set; }

Copy link
Member Author

Choose a reason for hiding this comment

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

If so, it creates a new array every time.

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

It's not a test, we need a value

ValidBlockEnd = StateRoot.Index + validBlockEndThreshold,
Sender = Contract.CreateSignatureRedeemScript(verifiers[MyIndex]).ToScriptHash(),
Data = data,
Witness = null!
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Witness = null!
Witness = Witness.Empty

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, a value isn't needed here because it will be overwritten after the signature. Therefore, simply keeping null is sufficient. The reason for explicitly assigning null! is due to the requirement of the required keyword. My plan is to convert all these classes to immutable types in the future and only use builders to construct these immutable types. This will simplify the caching mechanism considerably.

@Wi1l-B0t Wi1l-B0t removed their assignment Nov 8, 2025
public abstract class Peer : UntypedActor, IWithUnboundedStash
{
public IStash Stash { get; set; }
public IStash Stash { get; set; } = null!;
Copy link
Member

Choose a reason for hiding this comment

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

This is weird, it's because the interface?

Suggested change
public IStash Stash { get; set; } = null!;
public IStash? Stash { get; set; }

Copy link
Member Author

Choose a reason for hiding this comment

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

The system will automatically assign a value to it after the constructor, so we can assume that it is definitely not null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants