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

Include verification offset inside ScriptHash #1961

Closed
wants to merge 4 commits into from

Conversation

shargon
Copy link
Member

@shargon shargon commented Sep 24, 2020

No description provided.

@shargon shargon mentioned this pull request Sep 24, 2020
@shargon shargon marked this pull request as ready for review September 24, 2020 14:42
@shargon
Copy link
Member Author

shargon commented Sep 24, 2020

I will fix the UT after the review

@roman-khimov
Copy link
Contributor

But why this or #1964? What's wrong with verify method having any kind of offset without affecting script hash?

@erikzhang
Copy link
Member

We can extend NefFile to include the manifest, and then use the hash of the NefFile as the deployed contract hash.

@shargon
Copy link
Member Author

shargon commented Sep 25, 2020

Do you want a different hash for the same script?

@shargon
Copy link
Member Author

shargon commented Sep 25, 2020

We can set the hash of the smart contract acording to the id

@erikzhang
Copy link
Member

If the ABI changes, the function of the contract changes too. And you can get a different hash of a script by adding a NOP at the end.

@shargon
Copy link
Member Author

shargon commented Sep 25, 2020

and what happens if the user update the manifest only, it should migrate the hash?

@erikzhang
Copy link
Member

and what happens if the user update the manifest only,

He must upload a new nef file.

@shargon
Copy link
Member Author

shargon commented Sep 25, 2020

ok, I will create a new pr according to this

@erikzhang
Copy link
Member

erikzhang commented Sep 25, 2020

We can create multiple PRs. The first is to modifying NefFile to include the manifest and remove the script hash.

@shargon
Copy link
Member Author

shargon commented Sep 30, 2020

Closed because there are a better solutions

@shargon shargon closed this Sep 30, 2020
@shargon shargon deleted the script-hash-with-offset branch September 30, 2020 09:41
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