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

[CLI] Integrate sui-test-validator into sui start #18204

Merged

Conversation

stefan-mysten
Copy link
Contributor

@stefan-mysten stefan-mysten commented Jun 12, 2024

Description

This PR integrates sui-test-validator binary into sui start, but keeps the crate for now before being deprecated.

  • the default behaviour of the existing sui start is preserved
  • the default sui-test-validator behaviour can be achieved by passing two flags: sui start --force-regenesis --with-faucet.
  • adds a sui-test-validator.sh bash script that is executable to keep backward compatibility with the previous binary
  • keeps the sui-test-validator binary for now.
  • changed default port for GraphQL to 9125

One difference in flag/options usage is that sui-test-validator used to start graphql service when the graphql-port arg was passed. To keep it consistent across all three services, they each have their own flag to use to start the service with default host and port, and a port can be provided to it as well:

  • --with-faucet
  • --with-indexer or --with-indexer=9124
  • --with-graphql or --with-graphql=9125

Previously, sui-test-validator exposed a few flags and options (see orig code here), which were migrated to sui start. There are two new flags in sui start, --with-graphql and --with-faucet, to make it easier to start these services with default values. Due to this, the bash script checks for multiple flags and their values and tries to ensure everything is passed correctly.

sui-test-validator implemented a simple faucet wrapper of the sui-faucet crate. This was moved into faucet.rs module in sui crate.

Given that there are three extra services that can be started, plus a configuration variable to start the network without a fullnode, here is an explanation of what each service requires:
graphql requires indexer - without an indexer, the GraphQL will have no data to serve. This will throw an error.
indexer requires fullnode - without a fullnode, the indexer will start ingesting mainnet data with default settings. The program throws an error here.

Test plan

Existing tests.


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI: Integrated sui-test-validator into sui start.

sui start can now be used to also start an indexer, a GraphQL service, or a faucet. Note that you need to build sui with --features indexer, and have installed libpq to start the indexer and GraphQL services. By default, this feature is turned off to preserve backward compatibility of sui start. Alternatively, you can download the latest release and use the sui-pg binary, which is built using the indexer feature.

  • the default behaviour of the existing sui start is preserved
  • the default sui-test-validator behaviour can be achieved by passing two flags: sui start --force-regenesis --with-faucet.
  • adds a sui-test-validator.sh bash script to sui/scripts folder that is executable to keep backward compatibility with the previous binary
  • Rust SDK:

@stefan-mysten stefan-mysten requested review from a team, amnn, wlmyng, emmazzz and suiwombat as code owners June 12, 2024 00:44
@stefan-mysten stefan-mysten requested review from pchrysochoidis and mystieanwaya and removed request for a team June 12, 2024 00:44
Copy link

vercel bot commented Jun 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2024 11:07pm

Copy link

vercel bot commented Jun 12, 2024

@stefan-mysten is attempting to deploy a commit to the Mysten Labs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@ronny-mysten ronny-mysten left a comment

Choose a reason for hiding this comment

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

Comments on content part. Looks good!

Copy link
Contributor

@awelc awelc left a comment

Choose a reason for hiding this comment

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

It looks like this PR does a lot more things than what is stated in the description (e.g., all the new options added, lot's of changes in faucet.rs). I would be great to get a bit more of a high-level description on what the implemented/modified features are

crates/sui-faucet/src/faucet/simple_faucet.rs Outdated Show resolved Hide resolved
@stefan-mysten
Copy link
Contributor Author

It looks like this PR does a lot more things than what is stated in the description (e.g., all the new options added, lot's of changes in faucet.rs). I would be great to get a bit more of a high-level description on what the implemented/modified features are

It looks like this PR does a lot more things than what is stated in the description (e.g., all the new options added, lot's of changes in faucet.rs). I would be great to get a bit more of a high-level description on what the implemented/modified features are

Good point Adam. I edited the top description. Just to also answer here, the new options added are actually migrated from sui-test-validator, and there are only two new flags: --with-faucet and --with-graphql. More details in the description.
I have not added any new features per-say, as the main goal was to keep compatibility with the commands as they are used today to not disrupt users.

@stefan-mysten
Copy link
Contributor Author

@johnjmartin @ebmifa we need to ensure two things in the builds :

  • the sui binary is not built and released with --features indexer or --all-features. From what I can tell, it's only in one place the binaries are built with cargo build --all-features, but those do not seem to be uploaded anywhere... I think. Could you please confirm that?
  • we need to build an additional binary and upload it to our releases, called sui-pg, which is built using cargo build --release --bin sui --features indexer, which has a hard dependency on libpq.

I think the changes here are sufficient, but would appreciate another pair of eyes.
https://github.com/MystenLabs/sui/pull/18204/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34

@amnn
Copy link
Member

amnn commented Jun 27, 2024

Any worries regarding cherry picking this to devnet and testnet branches?

I think we spoke about this offline, but we can't pick to these specific branches, because they track positions on existing release branches. So instead, we would need to pick to those release branches, but technically speaking, if you do that and you update the corresponding devnet/testnet tracking branch, you would be telling validators that they need to update their binaries.

@stefan-mysten
Copy link
Contributor Author

Any worries regarding cherry picking this to devnet and testnet branches?

I think we spoke about this offline, but we can't pick to these specific branches, because they track positions on existing release branches. So instead, we would need to pick to those release branches, but technically speaking, if you do that and you update the corresponding devnet/testnet tracking branch, you would be telling validators that they need to update their binaries.

Yes, I clarified with PE and have a plan. This will be cherry picked into v1.28-release, and when next releases happen Mon/Tue, we will finalize the rest of the needed changes.

@stefan-mysten stefan-mysten merged commit dca6bfe into MystenLabs:main Jun 27, 2024
40 of 43 checks passed
@stefan-mysten stefan-mysten deleted the refactor_sui_test_validator branch June 27, 2024 14:29
stefan-mysten added a commit to stefan-mysten/sui that referenced this pull request Jun 27, 2024
This PR integrates `sui-test-validator` binary into `sui start`, but
keeps the crate for now before being deprecated.
- the default behaviour of the existing `sui start` is preserved
- the default `sui-test-validator` behaviour can be achieved by passing
two flags: `sui start --force-regenesis --with-faucet`.
- adds a `sui-test-validator.sh` bash script that is executable to keep
backward compatibility with the previous binary
- keeps the `sui-test-validator` binary for now.
- changed default port for GraphQL to 9125

One difference in flag/options usage is that `sui-test-validator` used
to start `graphql` service when the `graphql-port` arg was passed. To
keep it consistent across all three services, they each have their own
flag to use to start the service with default host and port, and a port
can be provided to it as well:
- `--with-faucet` or `--with-faucet=9123`
- `--with-indexer` or `--with-indexer=9124`
- `--with-graphql` or `--with-graphql=9125`

Previously, `sui-test-validator` exposed a few flags and options (see
orig code
[here](https://github.com/MystenLabs/sui/blob/4f52e053f2869328d9fe62501853fbd93c8624d7/crates/sui-test-validator/src/main.rs#L30-L85)),
which were migrated to `sui start`. There are two new flags in `sui
start`, `--with-graphql` and `--with-faucet`, to make it easier to start
these services with default values. Due to this, the bash script checks
for multiple flags and their values and tries to ensure everything is
passed correctly.

`sui-test-validator` implemented a simple faucet wrapper of the
`sui-faucet` crate. This was moved into `faucet.rs` module in `sui`
crate.

Given that there are three extra services that can be started, plus a
configuration variable to start the network without a fullnode, here is
an explanation of what each service requires:
`graphql requires indexer` - without an indexer, the GraphQL will have
no data to serve. `--with-indexer` will be implied if `--with-graphql` is passed.
`indexer requires fullnode` - without a fullnode, the indexer will start
ingesting mainnet data with default settings. The program throws an
error here.

Existing tests.

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [x] CLI: Integrated `sui-test-validator` into `sui start`.

`sui start` can now be used to also start an indexer, a GraphQL service,
or a faucet. Note that you need to build `sui` with `--features
indexer`, and have installed `libpq` to start the indexer and GraphQL
services. By default, this feature is turned off to preserve backward
compatibility of `sui start`. Alternatively, you can download the latest
release and use the `sui-pg` binary, which is built using the `indexer`
feature.
- the default behaviour of the existing `sui start` is preserved
- the default `sui-test-validator` behaviour can be achieved by passing
two flags: `sui start --force-regenesis --with-faucet`.
- adds a `sui-test-validator.sh` bash script to `sui/scripts` folder
that is executable to keep backward compatibility with the previous
binary
- [ ] Rust SDK:
stefan-mysten added a commit that referenced this pull request Jun 27, 2024
…crate into `sui` to release v1.28 branch (#18436)

This PR integrates `sui-test-validator` binary into `sui start`, but
keeps the crate for now before being deprecated.
- the default behaviour of the existing `sui start` is preserved
- the default `sui-test-validator` behaviour can be achieved by passing
two flags: `sui start --force-regenesis --with-faucet`.
- adds a `sui-test-validator.sh` bash script that is executable to keep
backward compatibility with the previous binary
- keeps the `sui-test-validator` binary for now.
- changed default port for GraphQL to 9125

One difference in flag/options usage is that `sui-test-validator` used
to start `graphql` service when the `graphql-port` arg was passed. To
keep it consistent across all three services, they each have their own
flag to use to start the service with default host and port, and a port
can be provided to it as well:
- `--with-faucet` or `--with-faucet=9123`
- `--with-indexer` or `--with-indexer=9124`
- `--with-graphql` or `--with-graphql=9125`

Previously, `sui-test-validator` exposed a few flags and options (see
orig code

[here](https://github.com/MystenLabs/sui/blob/4f52e053f2869328d9fe62501853fbd93c8624d7/crates/sui-test-validator/src/main.rs#L30-L85)),
which were migrated to `sui start`. There are two new flags in `sui
start`, `--with-graphql` and `--with-faucet`, to make it easier to start
these services with default values. Due to this, the bash script checks
for multiple flags and their values and tries to ensure everything is
passed correctly.

`sui-test-validator` implemented a simple faucet wrapper of the
`sui-faucet` crate. This was moved into `faucet.rs` module in `sui`
crate.

Given that there are three extra services that can be started, plus a
configuration variable to start the network without a fullnode, here is
an explanation of what each service requires:
`graphql requires indexer` - without an indexer, the GraphQL will have
no data to serve. `--with-indexer` will be implied if `--with-graphql`
is passed. `indexer requires fullnode` - without a fullnode, the indexer
will start ingesting mainnet data with default settings. The program
throws an error here.

Existing tests.

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [x] CLI: Integrated `sui-test-validator` into `sui start`.

`sui start` can now be used to also start an indexer, a GraphQL service,
or a faucet. Note that you need to build `sui` with `--features
indexer`, and have installed `libpq` to start the indexer and GraphQL
services. By default, this feature is turned off to preserve backward
compatibility of `sui start`. Alternatively, you can download the latest
release and use the `sui-pg` binary, which is built using the `indexer`
feature.
- the default behaviour of the existing `sui start` is preserved
- the default `sui-test-validator` behaviour can be achieved by passing
two flags: `sui start --force-regenesis --with-faucet`.
- adds a `sui-test-validator.sh` bash script to `sui/scripts` folder
that is executable to keep backward compatibility with the previous
binary
- [ ] Rust SDK:
@stefan-mysten stefan-mysten mentioned this pull request Jun 28, 2024
7 tasks
stefan-mysten added a commit that referenced this pull request Jun 28, 2024
## Description 

Fixes a bug I introduced with #18204 in the bash script building
`sui-pg` binary.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
stefan-mysten added a commit to stefan-mysten/sui that referenced this pull request Jun 28, 2024
## Description 

Fixes a bug I introduced with MystenLabs#18204 in the bash script building
`sui-pg` binary.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
stefan-mysten added a commit that referenced this pull request Jun 28, 2024
## Description 

Fixes a bug I introduced with #18204 in the bash script building
`sui-pg` binary.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
stefan-mysten added a commit that referenced this pull request Jul 1, 2024
## Description 

Prior to #18204, `sui start` accepted a network config file when using
`--network.config`, and `sui-test-validator` accepted a config directory
when using the `--config-dir`, so we need to support both. That PR
introduces a bug as in it accepts only directories.

This PR fixes this issue and should close  #18468.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
stefan-mysten added a commit to stefan-mysten/sui that referenced this pull request Jul 1, 2024
…abs#18469)

## Description 

Prior to MystenLabs#18204, `sui start` accepted a network config file when using
`--network.config`, and `sui-test-validator` accepted a config directory
when using the `--config-dir`, so we need to support both. That PR
introduces a bug as in it accepts only directories.

This PR fixes this issue and should close  MystenLabs#18468.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
stefan-mysten added a commit that referenced this pull request Jul 1, 2024
## Description 
Cherry pick #18469 into release branch v1.28

Prior to #18204, `sui start` accepted a network config file when using
`--network.config`, and `sui-test-validator` accepted a config directory
when using the `--config-dir`, so we need to support both. That PR
introduces a bug as in it accepts only directories.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
stefan-mysten added a commit that referenced this pull request Jul 1, 2024
…ead of `sui-test-validator` (#18471)

## Description 

PR #18204 integrated `sui-test-validator` into `sui`. This PR updates
all relevant workflows and documentation to use `sui` binary with the
right options and flags instead of `sui-test-validator`.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
ebmifa added a commit that referenced this pull request Jul 11, 2024
stefan-mysten added a commit that referenced this pull request Jul 12, 2024
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## Description 

This PR integrates `sui-test-validator` binary into `sui start`, but
keeps the crate for now before being deprecated.
- the default behaviour of the existing `sui start` is preserved
- the default `sui-test-validator` behaviour can be achieved by passing
two flags: `sui start --force-regenesis --with-faucet`.
- adds a `sui-test-validator.sh` bash script that is executable to keep
backward compatibility with the previous binary
- keeps the `sui-test-validator` binary for now.
- changed default port for GraphQL to 9125

One difference in flag/options usage is that `sui-test-validator` used
to start `graphql` service when the `graphql-port` arg was passed. To
keep it consistent across all three services, they each have their own
flag to use to start the service with default host and port, and a port
can be provided to it as well:
- `--with-faucet` or `--with-faucet=9123`
- `--with-indexer` or `--with-indexer=9124`
- `--with-graphql` or `--with-graphql=9125`

Previously, `sui-test-validator` exposed a few flags and options (see
orig code
[here](https://github.com/MystenLabs/sui/blob/4f52e053f2869328d9fe62501853fbd93c8624d7/crates/sui-test-validator/src/main.rs#L30-L85)),
which were migrated to `sui start`. There are two new flags in `sui
start`, `--with-graphql` and `--with-faucet`, to make it easier to start
these services with default values. Due to this, the bash script checks
for multiple flags and their values and tries to ensure everything is
passed correctly.

`sui-test-validator` implemented a simple faucet wrapper of the
`sui-faucet` crate. This was moved into `faucet.rs` module in `sui`
crate.

Given that there are three extra services that can be started, plus a
configuration variable to start the network without a fullnode, here is
an explanation of what each service requires:
`graphql requires indexer` - without an indexer, the GraphQL will have
no data to serve. `--with-indexer` will be implied if `--with-graphql` is passed.
`indexer requires fullnode` - without a fullnode, the indexer will start
ingesting mainnet data with default settings. The program throws an
error here.

## Test plan 

Existing tests.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [x] CLI: Integrated `sui-test-validator` into `sui start`. 

`sui start` can now be used to also start an indexer, a GraphQL service,
or a faucet. Note that you need to build `sui` with `--features
indexer`, and have installed `libpq` to start the indexer and GraphQL
services. By default, this feature is turned off to preserve backward
compatibility of `sui start`. Alternatively, you can download the latest
release and use the `sui-pg` binary, which is built using the `indexer`
feature.
- the default behaviour of the existing `sui start` is preserved
- the default `sui-test-validator` behaviour can be achieved by passing
two flags: `sui start --force-regenesis --with-faucet`.
- adds a `sui-test-validator.sh` bash script to `sui/scripts` folder
that is executable to keep backward compatibility with the previous
binary
- [ ] Rust SDK:
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## Description 

Fixes a bug I introduced with MystenLabs#18204 in the bash script building
`sui-pg` binary.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
…abs#18469)

## Description 

Prior to MystenLabs#18204, `sui start` accepted a network config file when using
`--network.config`, and `sui-test-validator` accepted a config directory
when using the `--config-dir`, so we need to support both. That PR
introduces a bug as in it accepts only directories.

This PR fixes this issue and should close  MystenLabs#18468.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
…ead of `sui-test-validator` (MystenLabs#18471)

## Description 

PR MystenLabs#18204 integrated `sui-test-validator` into `sui`. This PR updates
all relevant workflows and documentation to use `sui` binary with the
right options and flags instead of `sui-test-validator`.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
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.

5 participants