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

UI for smart contract deployment #44

Merged
merged 20 commits into from
Jan 16, 2020
Merged

UI for smart contract deployment #44

merged 20 commits into from
Jan 16, 2020

Conversation

djnicholson
Copy link
Member

Allows any AVM file in the active workspace to be deployed to any Neo Express instance.

Some limitations:

  • Contracts can't (yet) be deployed to mainnet or testnet (I'll hook that up after Add "Wallet Explorer"; support NEP-6 wallets; support mainnet/testnet transfer/claim #40 is merged)
  • Contracts deployed this way won't appear in the invocation panel (this needs custom RPC endpoint to get list of deployed contracts neo-express#20 to be resolved, then corresponding changes in neo-visual-tracker)
  • Storage requirements are currently hardcoded (we can either add checkboxes in the UI, or make compiler changes so this information is in the contract ABI, holding off for now until we decide which approach to follow)
  • Return type and parameter types are currently hard-coded (these could probably be determined from the ABI)
  • Metadata (e.g. description, email, author, etc.) are all set to empty strings. This could be left this way, or we could add text fields to the UI, or we could update the compiler to extract this information from attributes in the smart contract source code and add it to the ABI.

devhawk
devhawk previously requested changes Jan 8, 2020
Copy link
Contributor

@devhawk devhawk left a comment

Choose a reason for hiding this comment

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

I compiled TestContract, but the Deploy Contract UI still reads "No compiled contracts (*.avm files) were found in the current workspace." even after I refresh and/or reopen the deploy contract UI

@devhawk devhawk assigned djnicholson and unassigned devhawk Jan 8, 2020
@devhawk
Copy link
Contributor

devhawk commented Jan 8, 2020

Metadata (e.g. description, email, author, etc.) are all set to empty strings. This could be left this way, or we could add text fields to the UI, or we could update the compiler to extract this information from attributes in the smart contract source code and add it to the ABI.

This has already been done but you'll need to build NEON yourself in the short term to get it.

Copy link
Contributor

@devhawk devhawk left a comment

Choose a reason for hiding this comment

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

Originally, I had a build issue that was preventing the panel from displaying correctly. After cleaning my repo and rebuilding both the extension and the test contract, the panel displays correctly but it still can't find the .avm file in the folder

@devhawk
Copy link
Contributor

devhawk commented Jan 8, 2020

Should we also have a way to right click on an AVM file to initiate deployment (probably do this in a separate task)

@djnicholson
Copy link
Member Author

Originally, I had a build issue that was preventing the panel from displaying correctly. After cleaning my repo and rebuilding both the extension and the test contract, the panel displays correctly but it still can't find the .avm file in the folder

From Discord:
devhawkToday at 7:49 PM when I hit refresh in the deploy contracts panel, I get the following in debug console of the extension instance of VSCode: Error parsing contractDetector.ts:21 Object Error: ENOENT: no such file or directory, open '/c:/Users/harry/Source/neo/seattle/visual-devtracker/sample-workspace/TestContract/bin/Debug/netstandard2.0/publish/TestContract.avm' at Object.openSync (fs.js:447:3) at Object.func (electron/js2c/asar.js:138:31) at Object.func [as openSync] (electron/js2c/asar.js:138:31) at Object.readFileSync (fs.js:349:35) at Object.<anonymous> (electron/js2c/asar.js:580:40) at Object.readFileSync (electron/js2c/asar.js:580:40) at new Contract (c:\Users\harry\Source\neo\seattle\visual-devtracker\src\contractDetector.ts:15:36) at c:\Users\harry\Source\neo\seattle\visual-devtracker\src\contractDetector.ts:51:29 at Array.map (<anonymous>) at ContractDetector.<anonymous> (c:\Users\harry\Source\neo\seattle\visual-devtracker\src\contractDetector.ts:51:18) at Generator.next (<anonymous>) at fulfilled (c:\Users\harry\Source\neo\seattle\visual-devtracker\out\contractDetector.js:5:58) looks like a windows specific issue with filepath

@devhawk
Copy link
Contributor

devhawk commented Jan 9, 2020

Still hitting error finding .avm files on windows. I went in and manually removed the prefix slash from fullpath parameter in Contract constructor and it worked fine.

Error: ENOENT: no such file or directory, open '/c:/Users/harry/Source/neo/seattle/visual-devtracker/sample-workspace/TestContract/bin/Debug/netstandard2.0/publish/TestContract.avm'
    at Object.openSync (fs.js:447:3)
    at Object.func (electron/js2c/asar.js:138:31)
    at Object.func [as openSync] (electron/js2c/asar.js:138:31)
    at Object.readFileSync (fs.js:349:35)
    at Object.<anonymous> (electron/js2c/asar.js:580:40)
    at Object.readFileSync (electron/js2c/asar.js:580:40)
    at new Contract (c:\Users\harry\Source\neo\seattle\visual-devtracker\src\contractDetector.ts:15:36)
    at c:\Users\harry\Source\neo\seattle\visual-devtracker\src\contractDetector.ts:51:29
    at Array.map (<anonymous>)
    at ContractDetector.<anonymous> (c:\Users\harry\Source\neo\seattle\visual-devtracker\src\contractDetector.ts:51:18)
    at Generator.next (<anonymous>)
    at fulfilled (c:\Users\harry\Source\neo\seattle\visual-devtracker\out\contractDetector.js:5:58)

@djnicholson
Copy link
Member Author

Still hitting error finding .avm files on windows. I went in and manually removed the prefix slash from fullpath parameter in Contract constructor and it worked fine.

FIxed now. I need to remember to use the fsPath property on Uri objects (instead of path); path works by accident on *nix systems but fsPath is the propery way of getting the corresponding file system path for a file:// URI.

@djnicholson
Copy link
Member Author

Originally, I had a build issue that was preventing the panel from displaying correctly. After cleaning my repo and rebuilding both the extension and the test contract, the panel displays correctly

This is a known limitation of the watch script. You need to restart it whenever new panels are added. This is probably Ok because adding a panel is rare, but we can open a separate issue if we want the watch script to be more intelligent.

@djnicholson
Copy link
Member Author

Should we also have a way to right click on an AVM file to initiate deployment (probably do this in a separate task)

Opened #56

@djnicholson
Copy link
Member Author

Done

@djnicholson
Copy link
Member Author

  • Contracts deployed this way won't appear in the invocation panel (this needs neo-project/neo-express#20 to be resolved, then corresponding changes in neo-visual-tracker)

Opened #57

@djnicholson
Copy link
Member Author

  • Storage requirements are currently hardcoded (we can either add checkboxes in the UI, or make compiler changes so this information is in the contract ABI, holding off for now until we decide which approach to follow)
  • Return type and parameter types are currently hard-coded (these could probably be determined from the ABI)
  • Metadata (e.g. description, email, author, etc.) are all set to empty strings. This could be left this way, or we could add text fields to the UI, or we could update the compiler to extract this information from attributes in the smart contract source code and add it to the ABI.

Opened #58

//
const sb = neon.default.create.scriptBuilder();
const script = sb
.emitPush(neon.default.u.str2hexstring('')) // description
Copy link
Contributor

@devhawk devhawk Jan 15, 2020

Choose a reason for hiding this comment

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

Contract metadata should be in the abi.json file as of NEON v2.6. If the metadata property is missing, using empty strings is a fine default, but these should be using abi.json if possible

.emitPush(neon.default.u.str2hexstring('')) // author
.emitPush(neon.default.u.str2hexstring('')) // code_version
.emitPush(neon.default.u.str2hexstring('')) // name
.emitPush(0x01) // storage: {none: 0x00, storage: 0x01, dynamic: 0x02, storage+dynamic:0x03}
Copy link
Contributor

Choose a reason for hiding this comment

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

storage and dynamic is part of the aformentioned contract metadata. If the metadata property is missing, prompt the user instead of assuming contracts use storage but arent dynamic

.emitPush(neon.default.u.str2hexstring('')) // code_version
.emitPush(neon.default.u.str2hexstring('')) // name
.emitPush(0x01) // storage: {none: 0x00, storage: 0x01, dynamic: 0x02, storage+dynamic:0x03}
.emitPush('05') // return type - see https://docs.neo.org/docs/en-us/sc/deploy/Parameter.html
Copy link
Contributor

Choose a reason for hiding this comment

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

This information is in the abi.json file. If the abi.json file is missing, fail deployment rather than assuming return/parameter types

Copy link
Contributor

@devhawk devhawk left a comment

Choose a reason for hiding this comment

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

Are you planning on reading the abi.json file before completing this PR?

@devhawk devhawk assigned djnicholson and unassigned devhawk Jan 15, 2020
@djnicholson
Copy link
Member Author

Are you planning on reading the abi.json file before completing this PR?

After. I've already got issues created for the follow-on items (see above).

@devhawk
Copy link
Contributor

devhawk commented Jan 15, 2020

Are you planning on reading the abi.json file before completing this PR?
After. I've already got issues created for the follow-on items (see above).

These need to get done before we ship, but totally cool to separate out into multiple tasks

@djnicholson djnicholson merged commit a0850b6 into master Jan 16, 2020
@djnicholson djnicholson deleted the deploycontract branch January 16, 2020 13:19
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.

None yet

2 participants