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

Change smart contract hash computation #2240

Merged
merged 9 commits into from
Jan 19, 2021

Conversation

shargon
Copy link
Member

@shargon shargon commented Jan 15, 2021

Close #2233

Idea comes from : #2233 (comment)

@shargon shargon requested a review from erikzhang January 15, 2021 10:03
@roman-khimov
Copy link
Contributor

It'll work, but I'm still a little worried about these contracts having the same name. It'd be nice to get az, buki, vedi, etc in the end instead of seven alphabet contracts.

@@ -120,11 +121,30 @@ public IEnumerable<ContractState> ListContracts(StoreView snapshot)
return snapshot.Storages.Find(listContractsPrefix).Select(kvp => kvp.Value.GetInteroperable<ContractState>());
}

[ContractMethod(0, CallFlags.WriteStates | CallFlags.AllowNotify)]
private ContractState CreateChild(ApplicationEngine engine, StackItem data)
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to create this method, right? We can create a new child contract in contract/script directly, by this following way.

var contract = ContractManagement.GetContract(hash)
ContractManagement.Deploy(contract.nef, contract.manifest, data)

Copy link
Member

@erikzhang erikzhang Jan 15, 2021

Choose a reason for hiding this comment

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

The problem is that the sender of the transaction could be the same. Then it will generate the same hash.

Copy link
Contributor

Choose a reason for hiding this comment

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

@erikzhang
Copy link
Member

Now do we still need CreateChild()? We can use GetContract() to get the manifest, replace the name and then deploy the new contract.

@shargon
Copy link
Member Author

shargon commented Jan 15, 2021

Now do we still need CreateChild()? We can use GetContract() to get the manifest, replace the name and then deploy the new contract.

Agree it's a very specific case, and now it can be solved.

@shargon shargon changed the title CreateChild Change smart contract hash computation Jan 15, 2021
@roman-khimov
Copy link
Contributor

roman-khimov commented Jan 15, 2021

replace the name

This is the tricky part, because manifest is returned as a JSON string from GetContract().

@erikzhang
Copy link
Member

This is the tricky part, because manifest is returned as a JSON string from GetContract().

Use System.Json.Serialize and System.Json.Deserialize?

@Tommo-L
Copy link
Contributor

Tommo-L commented Jan 15, 2021

replace the name

This is the tricky part, because manifest is returned as a JSON string from GetContract().

Use #2237 to fix it? Create a new deploy method:

deploy(Nef nef, Manifest manifest, StackItem data)

@superboyiii superboyiii mentioned this pull request Jan 18, 2021
36 tasks
@Tommo-L
Copy link
Contributor

Tommo-L commented Jan 18, 2021

Or create a new method, like ContractManagement.Clone/New(ContractState origin, string name) ?

@shargon
Copy link
Member Author

shargon commented Jan 18, 2021

Or create a new method, like ContractManagement.Clone/New(ContractState origin, string name) ?

I think that change the name it's easy with the json syscalls

@shargon shargon merged commit 5f17eee into neo-project:master Jan 19, 2021
@shargon shargon deleted the create-child branch January 19, 2021 09:45
{
using var sb = new ScriptBuilder();
sb.Emit(OpCode.ABORT);
sb.EmitPush(sender);
sb.EmitPush(script);
sb.EmitPush(nefCheckSum);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn`t the checksum be calculated?

Assert.AreEqual("0xb8e95ff7b11c427c29355e3398722d97bd2ca069", Neo.SmartContract.Helper.GetContractHash(UInt160.Zero, nef.Script).ToString());
Assert.AreEqual("0x435c467b8e15cb9b1474ad7ee817ffdcfededef9", Neo.SmartContract.Helper.GetContractHash(UInt160.Parse("0xa400ff00ff00ff00ff00ff00ff00ff00ff00ff01"), nef.Script).ToString());
Assert.AreEqual("0x9b9628e4f1611af90e761eea8cc21372380c74b6", Neo.SmartContract.Helper.GetContractHash(UInt160.Zero, nef.CheckSum, "").ToString());
Assert.AreEqual("0x66eec404d86b918d084e62a29ac9990e3b6f4286", Neo.SmartContract.Helper.GetContractHash(UInt160.Parse("0xa400ff00ff00ff00ff00ff00ff00ff00ff00ff01"), nef.CheckSum, "").ToString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are they called "" instead of Neo and Gas?

roman-khimov added a commit to nspcc-dev/neo-go that referenced this pull request Jan 22, 2021
It allows to deploy the same NEF using one sender and get different contract
hashes. See neo-project/neo#2240.
cloud8little pushed a commit to cloud8little/neo that referenced this pull request Jan 24, 2021
* CreateChild

* Change to checkSum

* Remove nonce

* Change manifest to name

* Remove new method

* Clean changes
ixje added a commit to CityOfZion/neo-mamba that referenced this pull request Feb 16, 2021
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.

Contract factory example
5 participants