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

Script header #903

Merged
merged 30 commits into from
Jul 29, 2019
Merged

Script header #903

merged 30 commits into from
Jul 29, 2019

Conversation

shargon
Copy link
Member

@shargon shargon commented Jul 9, 2019

Close #889

@shargon shargon requested a review from erikzhang July 9, 2019 16:59
@codecov-io
Copy link

codecov-io commented Jul 11, 2019

Codecov Report

Merging #903 into master will increase coverage by 0.12%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
+ Coverage   52.05%   52.17%   +0.12%     
==========================================
  Files         178      179       +1     
  Lines       12662    12707      +45     
==========================================
+ Hits         6591     6630      +39     
- Misses       6071     6077       +6
Impacted Files Coverage Δ
neo/SmartContract/NefFile.cs 86.66% <86.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9f46bc...0d7f22e. Read the comment docs.

neo/SmartContract/ScriptFile.cs Outdated Show resolved Hide resolved
neo/SmartContract/ScriptFile.cs Outdated Show resolved Hide resolved
neo/SmartContract/ScriptFile.cs Outdated Show resolved Hide resolved
neo/SmartContract/InteropService.NEO.cs Outdated Show resolved Hide resolved
public void Serialize(BinaryWriter writer)
{
writer.Write((int)Magic);
writer.WriteFixedString(Compiler, 32);
Copy link
Contributor

Choose a reason for hiding this comment

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

32 bytes compiler? isn't it much?

Copy link
Member

Choose a reason for hiding this comment

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

Microsoft Visual Studio 2019

It consumes 28 bytes.

Copy link
Contributor

@igormcoelho igormcoelho left a comment

Choose a reason for hiding this comment

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

must see how to load this, and if these prefixes aren't too much for a binary nvm.

/// <summary>
/// Version
/// </summary>
public Version Version { get; set; }
Copy link
Contributor

@igormcoelho igormcoelho Jul 17, 2019

Choose a reason for hiding this comment

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

Which version is this? Neo (core) version? NeoVM version? Perhaps both?
But since every Neo (core) release carries a specific NeoVM version, it's fine to just use Neo (core) version I guess.

Or is it Compiler + Devpack version? hahaha too many versions... perhaps it's compiler related this, right? It would be good to have some minimal protocol version here, to differentiate between Neo2 and Neo3, and perhaps other affiliated Neo protocols.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Mayor, Minor, Build, Version)

shargon and others added 2 commits July 19, 2019 13:05
Co-Authored-By: Igor Machado Coelho <igor.machado@gmail.com>
@erikzhang
Copy link
Member

Also we can add here the description, author, email (if we will request this in the deploy)

Now we don't request these in the deployment.

@shargon
Copy link
Member Author

shargon commented Jul 22, 2019

If we want to accept ONT file format, is easier with an enum

@erikzhang
Copy link
Member

If we want to accept ONT file format, is easier with an enum

For ONT, we have System.Runtime.Platform.

@erikzhang
Copy link
Member

erikzhang commented Jul 23, 2019

Should we move it to neo-vm?

@shargon
Copy link
Member Author

shargon commented Jul 23, 2019

If is moved to neo-vm we need to remove the hash

@shargon
Copy link
Member Author

shargon commented Jul 26, 2019

i think that we should add the ABI, are you agree @erikzhang?

@erikzhang
Copy link
Member

i think that we should add the ABI

I prefer ABI in a separate file.

@shargon shargon merged commit f4adfac into neo-project:master Jul 29, 2019
@shargon shargon deleted the script-header branch July 29, 2019 05:25
@erikzhang erikzhang added this to the NEO 3.0 milestone Jul 29, 2019
@vncoelho
Copy link
Member

Here we go!

Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
* add big and little endian usage

* Update endian.md

* Update endian.md

* Update endian.md

* Update endian.md

* Update endian.md
Tommo-L pushed a commit to Tommo-L/neo that referenced this pull request Jun 22, 2020
* Script header

* Script header draft

* Clean files

* Typo

* Rename class and magic header

* Clean hash

* UT

* Erik's suggestions

* Change name

* Typo

* Check magic

* 64 to 32

* Nef file only for neo-cli

* Real Checksum

* Clean

* Clean again

* Remove code

* Update neo/SmartContract/NefFile.cs

Co-Authored-By: Igor Machado Coelho <igor.machado@gmail.com>

* Enum to const

* Remove unnecessary change

* Apply Erik's suggestions
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.

Script header for multiple virtual machines
5 participants