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.

Sign up for GitHub

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

Jump to bottom

Update contract without change the hash #2044

Merged
merged 59 commits into from
Dec 1, 2020
Merged

Update contract without change the hash #2044

merged 59 commits into from
Dec 1, 2020

Conversation

shargon
Copy link
Member

@shargon shargon commented Nov 3, 2020 •
edited
Loading

Alternative to #1973

Related to #1961 (comment)
Produce changes in neo-project/proposals#121 (review)
Close #2002 #2021 #1973

@shargon shargon requested a review from erikzhang November 3, 2020 12:04
@superboyiii superboyiii mentioned this pull request Nov 4, 2020
Closed
44 tasks
@superboyiii
Copy link
Member

@erikzhang Need your review. It's important to Preview4. Only several days left before code frozen.

@shargon
Copy link
Member Author

shargon commented Nov 4, 2020

@superboyiii could your team test it?

@Qiao-Jin
Copy link
Contributor

Qiao-Jin commented Nov 5, 2020

It seems that version is only used for observation. Can we drop it?

@superboyiii
Copy link
Member

@superboyiii could your team test it?

@ProDog will test this tommorrow.

@shargon
Copy link
Member Author

shargon commented Nov 5, 2020

It seems that version is only used for observation. Can we drop it?

You don't think that it's a useful information only with 4 bytes?

erikzhang
erikzhang reviewed Nov 6, 2020
View reviewed changes
src/neo/Ledger/ContractState.cs Outdated Show resolved Hide resolved
src/neo/Ledger/ContractState.cs Outdated Show resolved Hide resolved
src/neo/SmartContract/ApplicationEngine.Contract.cs Outdated Show resolved Hide resolved
src/neo/SmartContract/NefFile.cs Outdated Show resolved Hide resolved
@ProDog
Copy link
Contributor

ProDog commented Nov 6, 2020

Same error when deploy contract: #1973 (comment)

@superboyiii
Copy link
Member

superboyiii commented Nov 6, 2020 •
edited
Loading

@shargon Could you check this: neo-project/neo-devpack-dotnet#378 (comment)

@shargon
Copy link
Member Author

shargon commented Nov 7, 2020 •
edited
Loading

@superboyiii Please use neo-project/neo-node#676 and neo-project/neo-devpack-dotnet#388

This only require to change the hash in the manifest.

@shargon
Copy link
Member Author

shargon commented Nov 7, 2020

@erikzhang now dev-pack always generate the abi with the current scriptHash, how can we tell him the old scriptHash, because update will require the old one in the manifest.

@shargon shargon mentioned this pull request Nov 7, 2020
Closed
@erikzhang
Copy link
Member

now dev-pack always generate the abi with the current scriptHash

Maybe we can remove the hash from abi?

roman-khimov added a commit to nspcc-dev/neo-go that referenced this pull request Nov 26, 2020
@roman-khimov
*: introduce stable contract hashes
3ac2303 
Follow neo-project/neo#2044.
@superboyiii superboyiii mentioned this pull request Nov 27, 2020
Merged
erikzhang
erikzhang reviewed Nov 27, 2020
View reviewed changes
src/neo/SmartContract/NefFile.cs
/// Script
/// </summary>
public byte[] Script { get; set; }
public const int MaxScriptLength = 1024 * 1024;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that should it be smaller then 1 MB?

Copy link
Member Author

@shargon shargon Nov 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

We can remove it and check PayloadMaxSize they will pay for it

src/neo/SmartContract/NefFile.cs Outdated Show resolved Hide resolved
src/neo/Wallets/Wallet.cs Show resolved Hide resolved
@roman-khimov roman-khimov mentioned this pull request Nov 27, 2020
Closed
roman-khimov added a commit to nspcc-dev/neo-go that referenced this pull request Nov 27, 2020
@roman-khimov
*: introduce stable contract hashes
1cf1fe5 
Follow neo-project/neo#2044.
@Tommo-L
Copy link
Contributor

Tommo-L commented Nov 30, 2020

UT failed

@superboyiii
Copy link
Member

Merge? I've retested it and it works well.

@shargon shargon merged commit 6b139e3 into neo-project:master Dec 1, 2020
@shargon shargon deleted the update-no-hash-change branch December 1, 2020 09:44
@luc10921 luc10921 mentioned this pull request Dec 1, 2020
Merged
This was referenced Dec 2, 2020
Closed
Merged
Merged
Merged
ShawnYun pushed a commit to ShawnYun/neo that referenced this pull request Jan 8, 2021
@shargon @erikzhang
Update contract without change the hash (neo-project#2044)
624f6e9 
* Update without hash change

* Specify version

* Update ContractState.cs

* Update ApplicationEngine.Contract.cs

* Update ApplicationEngine.Contract.cs

* Update ApplicationEngine.Contract.cs

* Change to ushort

* Return Id

* Update ContractState.cs

* Remove hash_new check

* Update ApplicationEngine.Contract.cs

* Fix UT

* Remove Hash from ABI and change hash strategy

* Clean code

* Remove MaxLength

* Change script size verification to nefFile

* Rename Version to UpdateCounter

* Rename ContractState.ScriptHash to Hash

* Some Erik's suggestions

* Some Erik's suggestions

* Move CanCall

* Remove Script hash from NefFile

* Move to Helper

* Simplify ContractState

* Erik's review

* NefFile.GetHash()

* Remove double check

* Some fixes

* Some fixes

* Reduce changes

* Move GetContractHash to Helper

* Use GetContractHash for native contracts

* Fix UT paritially

* Some fixes

* Fix UT

* Rename parameters

* Update NefFile.cs

* Update NefFile.cs

* Change version to string

* Increase to 32

* Fix

* Fix UT

Co-authored-by: Erik Zhang <[email protected]>
@roman-khimov roman-khimov mentioned this pull request Jan 14, 2021
Closed
cloud8little pushed a commit to cloud8little/neo that referenced this pull request Jan 24, 2021
@shargon @erikzhang @cloud8little
Update contract without change the hash (neo-project#2044)
c0a3840 
* Update without hash change

* Specify version

* Update ContractState.cs

* Update ApplicationEngine.Contract.cs

* Update ApplicationEngine.Contract.cs

* Update ApplicationEngine.Contract.cs

* Change to ushort

* Return Id

* Update ContractState.cs

* Remove hash_new check

* Update ApplicationEngine.Contract.cs

* Fix UT

* Remove Hash from ABI and change hash strategy

* Clean code

* Remove MaxLength

* Change script size verification to nefFile

* Rename Version to UpdateCounter

* Rename ContractState.ScriptHash to Hash

* Some Erik's suggestions

* Some Erik's suggestions

* Move CanCall

* Remove Script hash from NefFile

* Move to Helper

* Simplify ContractState

* Erik's review

* NefFile.GetHash()

* Remove double check

* Some fixes

* Some fixes

* Reduce changes

* Move GetContractHash to Helper

* Use GetContractHash for native contracts

* Fix UT paritially

* Some fixes

* Fix UT

* Rename parameters

* Update NefFile.cs

* Update NefFile.cs

* Change version to string

* Increase to 32

* Fix

* Fix UT

Co-authored-by: Erik Zhang <[email protected]>
ixje added a commit to CityOfZion/neo-mamba that referenced this pull request Feb 2, 2021
@ixje
https://github.com/neo-project/neo/pull/2044
23e85ad 
stopped trying to fix tests, they're changing too much (also in the future)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tommo-L Tommo-L Tommo-L left review comments

@roman-khimov roman-khimov roman-khimov left review comments

@vncoelho vncoelho vncoelho left review comments

@ProDog ProDog ProDog left review comments

@erikzhang erikzhang erikzhang approved these changes

Assignees
No one assigned
Labels
Backward-Incompatible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Increase ContractManifest.MaxLength
9 participants
@shargon @superboyiii @Qiao-Jin @ProDog @erikzhang @Tommo-L @roman-khimov @cloud8little @vncoelho