Skip to content

Conversation

@Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Jul 9, 2025

Summary

This PR introduces a production-ready deployment toolkit for Neo smart contracts, complete with deterministic RPC shims and exhaustive unit coverage.

Highlights

  • DeploymentToolkit now enforces safe RPC usage, confirmation policies, and state snapshotting for manifest-driven deployments.
  • ✅ Rich configuration surface (SetNetwork, ConfigureOptions, SetWifKey) with disposal/validation guards to prevent misuse.
  • ✅ Helper infrastructure (QueueRpcClientFactory, StubRpcClient) enabling hermetic tests without live RPC endpoints.
  • ✅ Expanded unit suite (39 tests, no skips) plus deterministic JSON-RPC handlers replacing formerly skipped "integration" coverage.

Example Usage

var toolkit = new DeploymentToolkit()
    .SetNetwork("testnet")
    .SetWifKey(wif)
    .ConfigureOptions(o => o.WaitForConfirmation = true);

var result = await toolkit.DeployArtifactsAsync(
    nefPath,
    manifestPath,
    initParams: new object?[] { "admin", 1 },
    waitForConfirmation: true);

Console.WriteLine($"Contract hash: {result.ContractHash}");

Testing

  • dotnet test tests/Neo.SmartContract.Deploy.UnitTests/Neo.SmartContract.Deploy.UnitTests.csproj
  • All tests green; no external RPC dependency required.

Follow-ups

  • Future PRs will hook this toolkit into CLI workflows and add manifest-driven batch deployment ergonomics.

erikzhang and others added 2 commits June 16, 2025 21:35
* SafeAttribute supports in properties

* Update Nep11Token.cs

* Safe setters are not allowed
- Add DeploymentToolkit class with basic deployment API
- Implement network configuration support (mainnet, testnet, local, custom RPC)
- Add WIF key support for transaction signing
- Create comprehensive project structure with interfaces and models
- Add configuration management with JSON and environment variable support
- Include 16 unit tests covering all basic functionality
- Update README.md with deployment documentation and usage examples

This establishes the foundation for Neo smart contract deployment capabilities
without implementation details, providing a clean base for future enhancements.
…rence

- Fix whitespace formatting issues in deployment toolkit files
- Add missing final newlines to all source files
- Remove duplicate Microsoft.NET.Test.Sdk package reference in test project
- All 16 unit tests continue to pass
- Code formatting now passes GitHub Actions checks
@Jim8y Jim8y force-pushed the pr1-core-deployment-framework branch from 2f05ca7 to ce28e18 Compare July 9, 2025 11:00
@Jim8y Jim8y force-pushed the pr1-core-deployment-framework branch from ce28e18 to ef94799 Compare July 9, 2025 11:06
Jim8y added 8 commits July 9, 2025 21:07
- Remove Network property from NetworkConfiguration as it depends on RpcUrl
- Change NetworkMagic to nullable and retrieve from RPC when not configured
- Add logic to fetch NetworkMagic from RPC using GetVersionAsync()
- Remove unnecessary Compile Update section from csproj file
- Update unit tests to match new behavior
- Add tests to verify network magic can be retrieved from RPC
- Add tests for network configuration priority (specific > global > RPC)
- Add tests for SetNetwork with various inputs and edge cases
- Add tests to ensure proper RPC URL configuration for known networks
- Document expected network magic values for mainnet and testnet
- Change testnet RPC URL from testnet1.neo.coz.io to testnet.rpc.ngd.network
- Update all test assertions to use the new NGD testnet URL
- Add integration tests to verify RPC connectivity (skipped by default)
- Ensure network magic retrieval works with NGD testnet endpoint
- Change from testnet.rpc.ngd.network to testnet.ngd.network
- Update all test assertions to use the correct URL
- Change testnet RPC URL to http://seed2t5.neo.org:20332
- Update all test assertions to use the Neo seed node URL
- Update integration test names to reflect Neo testnet
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Hi @Jim8y

My suggestion is that if this is able to Deploy and test, so, the examples we have here should be deployed during tests and we could verify states.
That would be a good test that has a real flow behind.

On the other hand, I see some separation between this repo and the framework being pushed here.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jul 9, 2025

Hi @Jim8y

My suggestion is that if this is able to Deploy and test, so, the examples we have here should be deployed during tests and we could verify states. That would be a good test that has a real flow behind.

On the other hand, I see some separation between this repo and the framework being pushed here.

its hard to balance, smaller prs for easier to review, but lacks complete functionality, complete tookit too large for reviewing. yet if you wish to review as a whole, you can check the full pr. bellow is the deployed example contracts on testnet.

Latest Deployment (TestNet)

Contract Address Transaction Hash Network
TokenContract 0x2db2dce76b4a7f8116ecfae0d819e7099cb3a256 0x29397173e0e5ad93010f721ff6f786171e32e84f9bc44067072df332853e9554 TestNet
NFTContract 0x8699c5d074fc27cdbd7caec486387c1a29300536 0xc0b8c4b7dd68525da2adc39455252f32ac884947aeaba387c6ce20fa017952b7 TestNet
GovernanceContract 0xa3db58df3764610e43f3fda0c7b8633636c6c147 0x43d4a849396ddaf6b9917a3aeb30e49929a15fa32ebf51b1ab4b2d9e46dcf5a8 TestNet

Deployment Details:

  • Deployer Address: NTmHjwiadq4g3VHpJ5FQigQcD4fF5m8TyX (0x0c3146e78efc42bfb7d4cc2e06e3efd063c01c56)
  • Network: NEO TestNet
  • RPC Endpoint: https://testnet1.neo.coz.io:443
  • Total GAS Consumed: ~30.00233163 GAS
  • Deployment Mode: Multi-contract with dependencies

@vncoelho
Copy link
Member

vncoelho commented Jul 9, 2025

I see @Jim8y, but why this needed to be here in Devpack?
Isn't this focussed on the C# compiler.

What I see in this pr is like a framework for interacting with the blockchain.

@vncoelho
Copy link
Member

vncoelho commented Jul 9, 2025

I see @Jim8y, but why this needed to be here in Devpack? Isn't this focussed on the C# compiler.

What I see in this pr is like a framework for interacting with the blockchain.

Maybe even the Core itself is better than here

@Jim8y
Copy link
Contributor Author

Jim8y commented Jul 9, 2025

I see @Jim8y, but why this needed to be here in Devpack? Isn't this focussed on the C# compiler.
What I see in this pr is like a framework for interacting with the blockchain.

Maybe even the Core itself is better than here

implementing, testing, deploying is a complete workflow, with this, developer can do everfything easily all together. they dont need to download and install and sync neo-cli, or searching for some unknown deployment methods. All of these is for a better development experience.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jul 11, 2025

@shargon all your concern and suggestion have being resolved.

/// <summary>
/// Wallet configuration settings
/// </summary>
public class WalletConfiguration
Copy link
Member

Choose a reason for hiding this comment

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

One class per file please

…ffolding; add DeploymentArtifactsDemo example; fix RpcStore OnNewSnapshot; docs update
@shargon shargon changed the base branch from master to dev September 12, 2025 09:44
@shargon
Copy link
Member

shargon commented Sep 12, 2025

@Jim8y Could you solve the conflicts?

@Jim8y Jim8y changed the title feat: add core Neo smart contract deployment framework feat: core deployment toolkit with tested RPC integration Oct 24, 2025
@shargon shargon requested a review from ajara87 October 27, 2025 20:50
Comment on lines +36 to +39
public static NetworkProfile MainNet { get; } = new("mainnet", "https://mainnet1.neo.coz.io:443", 860833102, 0x35);
public static NetworkProfile TestNet { get; } = new("testnet", "http://seed2t5.neo.org:20332", 894710606, 0x35);
public static NetworkProfile Local { get; } = new("local", "http://localhost:50012");
public static NetworkProfile Private { get; } = new("private", "http://localhost:50012");
Copy link
Member

Choose a reason for hiding this comment

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

I think that the AddressVersion and NetworkMagic should be taken from an rpc request, we are not able to sign a different network by rpc

Jim8y added 4 commits November 5, 2025 20:36
- README: document new signer hooks and standardise init param example
- DeploymentArtifactsRequest.cs: add signer + custom transaction signer hooks
- DeploymentOptions.cs: add signer provider, transaction signer plumbing
@Jim8y
Copy link
Contributor Author

Jim8y commented Nov 8, 2025

Thanks for the detailed write-up and examples. I still have a few blocking concerns before this can merge.

  1. Scope / repo ownership – this PR adds an entire deployment framework (brand-new Neo.SmartContract.Deploy project plus example + extensive tests) to the compiler repo. Devpack currently ships the compiler/test harness and is consumed inside other IDE tooling. Dropping ~1.6k lines of runtime deployment/client code here creates a new product surface with its own lifecycle, dependencies (Microsoft.Extensions.Configuration, RPC factories, WIF management, etc.) and release cadence. Can we keep this effort in a dedicated repository/package and leave the devpack focused on compilation? If the intent really is to own deployment tooling here, it needs a clear plan for packaging, versioning and long-term maintenance.

  2. Configuration loading from Directory.GetCurrentDirectory()DeploymentToolkit’s default ctor (see src/Neo.SmartContract.Deploy/DeploymentToolkit.cs:52-66) walks the current working directory and environment-specific appsettings.*.json. In library usage (MSBuild, dotnet CLI, editors) the current directory is unpredictable, so the toolkit may try to load the caller’s appsettings file or a random file on disk. Please require an explicit configuration root (or default to AppContext.BaseDirectory) so using the toolkit from scripts/tests doesn’t depend on process working directory.

  3. Plain-text WIF handling_wifKey is kept as a raw string field (DeploymentToolkit.cs:37, 765-804) and copied around by UseTemporaryWif. Disposing the toolkit never zeroes the string, so the private key stays live in managed memory. At minimum we should store a WalletAccount/KeyPair and zero the underlying byte array when we’re done; holding WIF text in a volatile string is an easy leak vector for crash dumps/logging. Could you refactor this so sensitive key material is contained inside WalletAccount/KeyPair instances and cleared/disposed when we stop using it?

Happy to re-review once those items are resolved.

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.

6 participants