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

code optimization #2958

Merged
merged 2 commits into from
Nov 10, 2023
Merged

code optimization #2958

merged 2 commits into from
Nov 10, 2023

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Nov 10, 2023

This pr does the following three things:

  1. add hashOrPubkey exception message.
  2. remove else statement since it is not working.
  3. remove unnecessary variable hashes_for_verifying.

@Jim8y Jim8y requested review from AnnaShaleva and shargon November 10, 2023 08:01
shargon
shargon previously approved these changes Nov 10, 2023
@shargon shargon self-requested a review November 10, 2023 08:31
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.

@Liaojinghui The coverage is decreased :)

@Jim8y
Copy link
Contributor Author

Jim8y commented Nov 10, 2023

@Liaojinghui The coverage is decreased :)

I started to hate those rules, but i know we have to. I will check and try to fix it.

@Jim8y
Copy link
Contributor Author

Jim8y commented Nov 10, 2023

@shargon

@shargon shargon merged commit 64fecc4 into neo-project:master Nov 10, 2023
1 check passed
@Jim8y Jim8y deleted the code-optimization branch November 10, 2023 10:44
var engine = GetEngine();

engine.CheckWitness(pubkey.EncodePoint(true)).Should().BeFalse();
engine.CheckWitness(pubkey.EncodePoint(true)).Should().BeFalse();
Copy link
Member

Choose a reason for hiding this comment

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

Two checks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can we just leave it this way for now? My bad, did not notice it at all.

Copy link
Member

Choose a reason for hiding this comment

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

Just fix in a new pr

Jim8y added a commit to Jim8y/neo that referenced this pull request Dec 31, 2023
* master: (30 commits)
  Set project as nullable (neo-project#3042)
  Fix: fix equal (neo-project#3028)
  Added README to packages (neo-project#3026)
  Nuget MyGet Fix (neo-project#3031)
  Add: print out the stack (neo-project#3033)
  fixed myget (neo-project#3029)
  Fixed MyGet Workflow (neo-project#3027)
  Package icons - hotfix (neo-project#3022)
  Nuget Package Icon & Symbols (neo-project#3020)
  Fix warning (neo-project#3021)
  Neo-node Migration (neo-project#2990)
  Remove unnecessary default seedlist (neo-project#2980)
  Fix Neo VM target frameworks (neo-project#2989)
  Update Neo.VM location in README.md (neo-project#2988)
  Migrating Neo VM (neo-project#2970)
  3.6.2 (neo-project#2962)
  fix ut (neo-project#2959)
  Validate serialization during Contract deploy and Update (neo-project#2948)
  code optimization (neo-project#2958)
  check null scriptcontainer (neo-project#2953)
  ...
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