@erikzhang please take a look, I included only the Abi, the manifest it was thought to be modified in order to add some rights, but the abi should be changed only if the script change. So how can the developer change the manifest before create the nef file?

We can remove Abi from the manifest and add the Abi inside ContractState, what do you think?

Any update?

There is no way to compute the hash in the ABI. Maybe we should remove it.

There is no way to compute the hash in the ABI. Maybe we should remove it.

Done, but we need it in manifest for check if it's a valid call so I moved it to Manifest.

• neo/src/neo/SmartContract/Manifest/ContractManifest.cs

  Line 186 in 588cd98

  public bool IsValid(UInt160 hash)

• neo/src/neo/SmartContract/Manifest/ContractPermission.cs

  Line 74 in 588cd98

  if (!Contract.Hash.Equals(manifest.Hash)) return false;

@roman-khimov

If we're to go this route then maybe it's easier to put full manifest inside NEF, it'd simplify things a bit for deployment/updates and ContractState will have it too.

If you want to update only the manifest, for change rights, you will need a new script?

We delete them on update or destroy. Imagine all of the above use cases with an ID instead of a hash (for calls that would be 4-byte IDs, for witnesses 20-byte with lots of zeroes). The only problematic case I see is IsStandard, but even that is probably OK (and this patch breaks it too anyway).

I like the idea of use the id instead of the hash in some cases (for optimization), but how it solves the main problem?

If you want to update only the manifest, for change rights, you will need a new script?

I thought we're to change from script hash to NEF hash effectively (at least that's my interpretation of #1961 (comment)), thus change in the manifest changes the NEF and the hash. Am I missing something?

I like the idea of use the id instead of the hash in some cases (for optimization), but how it solves the main problem?

By the ability to differentiate script hashes from IDs? I mean regular scripts would have script hash as a signer, for example NTZJ91cwZWiad8uj8sLMF7X6j8Ed41Cim6 (41546d4fee9fe73744a08fc5055d76a88b17d853), this can't be contract ID, thus it must have a corresponding verification script attached and we won't look into the DB for it. Now NKuyBkoGdZZSLyPbJEetheRhMjezqzTxcW (0100000000000000000000000000000000000000) is the opposite, it can't be a hash (sure, technically it can, but good luck with that), so it must not have any verification script and we will take contract №1 to verify that. I know it sounds a bit fishy, but I can't find where this approach breaks.

Then we need to remove ABI from manifest.

@neo-project/ngd-shanghai please, could you test it?

I'll test this after neo-devpack-dotnet is updated.

Please take a look to neo-project/neo-devpack-dotnet#378

Please take a look to neo-project/neo-devpack-dotnet#378

Find CLI also should be modified: https://github.com/neo-project/neo-node/blob/90c7f396df2ac52c4ced880c433d286719e578ce/neo-cli/CLI/MainService.cs#L284

Compile it successfully and try to deploy it on neo-cli, find magic unmatch...

Compile it successfully and try to deploy it on neo-cli, find magic unmatch...

Did you use a nef file?

Compile it successfully and try to deploy it on neo-cli, find magic unmatch...

Did you use a nef file?

Yes, of course.

@superboyiii It's a new generated nef file? Could you send me by discord your nef and manifest?

I have a simpler idea. We don't need to modify the core, we can make the compiler add an instruction "return false;" before the generated contract to solve this problem.

@superboyiii It's a new generated nef file? Could you send me by discord your nef and manifest?

Sure. I upload the whole project here so that you could find the issue clearly. https://packet.azureedge.net/neochain/neo3/neo-PR#1973.zip
I'm using #1973, neo-project/neo-node#676 and neo-project/neo-devpack-dotnet#378. There's a privatenet inside and I'm using nep5 template for test. admin.json is the owner and password is 1. Nef and manifest are all in debug path of neo-cli which is an external node for test and you can rebuild it in ./neo-devpack-dotnet/templates/Template.NEP5.CSharp as well.

Compile it successfully and try to deploy it on neo-cli, find magic unmatch...

Yes, reader.ReadUInt32() was 2013333335 when I tested it.

Compile it successfully and try to deploy it on neo-cli, find magic unmatch...

Yes, reader.ReadUInt32() was 2013333335 when I tested it.

Yes, for me is 135056913, it's changing due to different sc. And it should always be 860243278.
@shargon

@superboyiii I will wait for #2021 because maybe we choose a different fix

@superboyiii I will wait for #2021 because maybe we choose a different fix

Fine. But I think this feature might be included in the version after Preview4 because not much time left for release. Do you think it's OK?

Fine. But I think this feature might be included in the version after Preview4 because not much time left for release. Do you think it's OK?

I think that it's the most important one in preview4 🤣

Fine. But I think this feature might be included in the version after Preview4 because not much time left for release. Do you think it's OK?

I think that it's the most important one in preview4 🤣

Then we have to delay Preview4 for several weeks. Only one week left.🤣

@superboyiii We only need to discuss #2021 and we can move foreward.

This or #2021 are very invasive things, so independent of what's going to be the accepted solution it needs as much testing as we could provide. And preview4 is exactly for that, testing in a wider setting, so I'd rather wait a little, but settle this before preview4.

Closed in favor of #2044