Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Allow smart contract verification #628

Merged
merged 20 commits into from
Aug 13, 2020
Merged

Allow smart contract verification #628

merged 20 commits into from
Aug 13, 2020

Conversation

shargon
Copy link
Member

@shargon shargon commented Jul 20, 2020

As seen in the talk with @lock9

  • Allow to see watch only address
  • Allow to use smart contract as sender
  • Remove unused method.

Require neo-project/neo#1800

Comment on lines 48 to 51
{
Signer[] signers = Array.Empty<Signer>();
if (signerAccounts != null && !NoWallet())
signers = CurrentWallet.GetAccounts().Where(p => !p.Lock && !p.WatchOnly && signerAccounts.Contains(p.ScriptHash)).Select(p => new Signer() { Account = p.ScriptHash, Scopes = WitnessScope.CalledByEntry }).ToArray();
signers = CurrentWallet.GetAccounts().Where(p => !p.Lock && signerAccounts.Contains(p.ScriptHash)).Select(p => new Signer() { Account = p.ScriptHash, Scopes = WitnessScope.CalledByEntry }).ToArray();
Copy link
Member

Choose a reason for hiding this comment

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

Why WatchOnly address can be signer?

Copy link
Member Author

@shargon shargon Jul 21, 2020

Choose a reason for hiding this comment

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

Smart contract withdrawal require a watch only address.

@shargon
Copy link
Member Author

shargon commented Jul 22, 2020

@lock9 could you confirm that it solve your problem?

@lock9
Copy link
Contributor

lock9 commented Jul 24, 2020

@shargon I can only test this tomorrow morning, by only checking the code I can't ensure it works

Co-authored-by: Luchuan <luchuan@ngd.neo.org>
@shargon
Copy link
Member Author

shargon commented Jul 24, 2020

If we want to allow to use smart contract as sender, we must modify neo-core first.

You should add as your smart contract as watch only first.

@ProDog
Copy link
Contributor

ProDog commented Jul 27, 2020

I have tested and it throw an exception, if I import watchonly a smart contract hash, because it's key is null.

neo> import watchonly 0xb6770c896f96f3d85381ec45a403f37d18f6d714
Address: NMpBQr7KMPoVd82LeJbVYnH35m8jxWvhZh

image

@shargon
Copy link
Member Author

shargon commented Jul 27, 2020

Yes, we can't know the pubkey

@meevee98
Copy link
Contributor

I've tried to withdraw gas from a smart contract by using the transfer command.
The validation is working well, until the moment the cli asks for relaying the transaction. If I relay, it fails to send the transaction.

image

As @Tommo-L has said, the neo-core must be changed as well. I've been researching and this fail is because the Wallet.Sign, that only has validation for multi signature or accounts with public key. Including a smart contract as a watch-only address doesn't include a public key, so a transaction signing fails.

https://github.com/neo-project/neo/blob/daa6e7a3b95524534107cfb6e174d9a52f3cd617/src/neo/Wallets/Wallet.cs#L414-L440

@shargon
Copy link
Member Author

shargon commented Jul 28, 2020

@meevee98 @Tommo-L please, could you check neo-project/neo#1800 ?

@shargon shargon changed the title Allow smart contract verify Allow smart contract verification Jul 28, 2020
@ProDog
Copy link
Contributor

ProDog commented Aug 3, 2020

I have tested it OK.
But I have a question, why must import watchonly first if add contract as verification? For developers, they would like to make it easier to use this, such as using the SDK to construct this transaction.

@shargon
Copy link
Member Author

shargon commented Aug 3, 2020

@ProDog now with neo-project/neo#1800 (comment) (neo-project/neo@ab971ce) it's not required to add the contract as watch-only.

@ProDog ProDog mentioned this pull request Aug 10, 2020
ProDog and others added 2 commits August 10, 2020 10:20
* add sender

* Adjust the order

* update param

* fix signers in invoke

* fix sender's order
@ProDog
Copy link
Contributor

ProDog commented Aug 12, 2020

@erikzhang this need to merge, please review it at sometime.

@erikzhang
Copy link
Member

Compilation error.

@shargon
Copy link
Member Author

shargon commented Aug 13, 2020

merge?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants