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

feat: Multinode P2P sync and POA ( Proof of Authority ) #1333

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 81 additions & 1 deletion gno.land/cmd/gnoland/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,88 @@
$> cd ./gno/gno.land
$> make install.gnoland

## Run `gnoland` full node
## Option 1: Run `gnoland` full node for local development

$> gnoland start

Afterward, you can interact with [`gnokey`](../gnokey) or launch a [`gnoweb`](../gnoweb) interface.


## Option 2: Run a node and sync with a Proof of Authority (POA) network
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Option 2: Run a node and sync with a Proof of Authority (POA) network
## Option 2: Run a full node and sync with a network

This is a valid instruction for running a full node on a network, whether it is PoA or not.


- gnoland init
Copy link
Member

Choose a reason for hiding this comment

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

I believe this step is unnecessary in option 2.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's needed to generate the $DAEMON_HOME (GNO_HOME) directory, with the config and data/


- Download genesis.json from a trusted source and save it to the root-dir/config directory.

- Get the peer's node id from the node you trust.

- Start the node

$> gnoland start --persistent "node_id@peer_ip_address:port" or add the persistent_peers value in the ./testdir/config/config.toml

## Option 3: Run a node as a Proof of Authority validator starting from genesis state

- Initialize the config and key files.

$> gnoland init

- Return the node info; we will need it to add to validator info in the genesis.json

$> gnoland node

Address: "g14t47gv3v2z3pc23g3zr39mnc99w2cplp0jhqvv"
Pubkey: "E5IFULgXFdS49ILgvPmO3/8chuSWfbqw3zYXaNEP+60="

Comment on lines +38 to +40
Copy link
Member

Choose a reason for hiding this comment

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

This output is not the correct format I get when running gnoland node, ex:

NodeID: g1et0nr2z32lhp6qyfqxdlenxmlw9h3auqy7fw7c

Copy link
Member

Choose a reason for hiding this comment

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

In cosmos-sdk, this is

gaiad tendermint show-node-id
gaiad tendermint show-address

I believe it should be 2 separate commands because most of the time, people use them with my action to $(gnoland tm2 show-address)

- Download genesis.json from a trusted source and save it to the root-dir/config directory.

- Add your validator to the genesis file.
Copy link
Member

Choose a reason for hiding this comment

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

If we want to retain the POA title in this PR, which I find confusing, I suggest revamping the example. Instead of just adding yourself as a validator, the example should emphasize coordinating with others and running the genesis validator add multiple times.

The current explanation is not clear. A possible improvement is to instruct validators to add their node ID to the shared genesis file (e.g., on a GitHub repo) and ensure they start with persistent node IDs for connectivity with the updated genesis file.

However, it's important to include a prominent warning that this approach is not actually PoA or Po something. It's simply a genesis with peers and no proof of anything except "being in the genesis."

Copy link
Member

Choose a reason for hiding this comment

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

This is the gentx system for validators to be able to add themself inside the genesis.json
cf: #1102


$> genesis validator add \
Copy link
Member

Choose a reason for hiding this comment

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

We could be more generic, with a gentx system, it could be use to add genesis validator, to setup account money, setup realms and packages ...

Copy link
Member

Choose a reason for hiding this comment

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

This genesis validator command will fail, since it's looking for a genesis.json locally by default, and not in the config folder (the path flag is missing here)

Copy link
Member

Choose a reason for hiding this comment

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

There needs to be a step where the genesis.json is generated

--address g14t47gv3v2z3pc23g3zr39mnc99w2cplp0jhqvv \
--pub-key E5IFULgXFdS49ILgvPmO3/8chuSWfbqw3zYXaNEP+60= \
Copy link
Member

Choose a reason for hiding this comment

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

The value for this flag is not good here, getting an error:
invalid validator public key, string not all lowercase or all uppercase

It's because genesis validator add expects a bech32 representation of the public key

--power 10 \
--name testvalidator2

- Share the genesis with all trusted validators.

- Get the peer's node id from the archive node you trust.

- Start the node

$> gnoland start --persistent "node_id@peer_ip_address:port"

or add the persistent_peers value in the ./testdir/config/config.toml


## Option 4: Run as an archive node starting from genesis state

It's recommended to have at least two POA validator nodes running as archive nodes to bootstrap the network.

Complete the steps in Option 4 and replace the last two steps with

- Retrive node id and give it trusted peers.

$> gnoland node
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$> gnoland node
$> gnoland init
$> gnoland node


- Start the node

$> gnoland start --prune "nothing"
Copy link
Member

Choose a reason for hiding this comment

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

Flag should be name --pruning as in the config file



## Reset `gnoland` node back to genesis state. It's suitable for running test node

$> gnoland unsafe-reset-all
Copy link
Member

Choose a reason for hiding this comment

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

Please add the flag --keep-addr-book

Question: In cosmos-sdk, this command is under gaiad tendermint unsafe-reset-all
Should we do the same ?

Usage:
  gaiad tendermint [command]

Available Commands:
  reset-state      Remove all the data and WAL
  show-address     Shows this node's tendermint validator consensus address
  show-node-id     Show this node's ID
  show-validator   Show this node's tendermint validator info
  unsafe-reset-all (unsafe) Remove all the data and WAL, reset this node's validator to genesis state
  version          Print tendermint libraries' version

Copy link
Member

Choose a reason for hiding this comment

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

or maybe gnoland tm2 ?


It removes the database and reset validator state back to genesis state but leaves the genesis.json and config.toml files unchanged.

The `unsafe-reset-all` command is labeled "unsafe" because:

1. It irreversibly deletes all node data, risking data loss.
2. It may lead to double signing or chain forks in production.
3. It resets the `priv_validator_state.json`, and can cause network disruption if uncoordinated.

## Reset `gnoland` node history back to genesis state.

It removes the datastore and keeps the validator state unchanged. It reduces the risk of double signing and chain fork when we sync history state from the genesis. The validator will not sign a block until the node has synced, passing the state where the validator stopped signing.

$> gnoland reset-state
Comment on lines +77 to +93
Copy link
Member

Choose a reason for hiding this comment

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

@albttx, what are your thoughts on these commands? Have you run them in production or on the testnet in the cosmos ecosystem? I want to understand which case these two commands handle, and I think maybe one command would suffice.

Copy link
Member

Choose a reason for hiding this comment

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

@moul I never used it before because it's new and i wasn't aware of it, but i might now use it often!

In tendermint, the more data it stored for a validator, the slower it is...
So we often restart from fresh install (prunned snapshot or state-sync) to have only latest block and be more efficient.
gnoland reset-state will be use in that case

55 changes: 55 additions & 0 deletions gno.land/cmd/gnoland/init.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package main

import (
"context"

"github.com/gnolang/gno/tm2/pkg/bft/privval"
"github.com/gnolang/gno/tm2/pkg/commands"
tmos "github.com/gnolang/gno/tm2/pkg/os"
"github.com/gnolang/gno/tm2/pkg/p2p"
)

// Display a node's persistent peer ID to the standard output.
func newInitCmd(bc baseCfg) *commands.Command {
cmd := commands.NewCommand(
commands.Metadata{
Name: "init",
ShortUsage: "init",
ShortHelp: "initialize gnoland node",
Copy link
Member

Choose a reason for hiding this comment

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

Currently, I believe it doesn't initialize a Gnoland node (the start command does that), but it does initialize a validator keypair. Perhaps the help message should provide more accurate information, or we could transfer all the initialization logic from start to this location to make the help message more relevant.

},
nil,
func(_ context.Context, args []string) error {
return execInit(bc)
},
)
return cmd
}

func execInit(bc baseCfg) error {
config := bc.tmConfig
// private validator
privValKeyFile := config.PrivValidatorKeyFile()
privValStateFile := config.PrivValidatorStateFile()
var pv *privval.FilePV
if tmos.FileExists(privValKeyFile) {
logger.Info("Found private validator", "keyFile", privValKeyFile,
Copy link
Member

Choose a reason for hiding this comment

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

What about panicking with the message "initialization already done" or something similar?

@albttx, we need your feedback on the initialization "UX" please.

"stateFile", privValStateFile)
} else {
pv = privval.GenFilePV(privValKeyFile, privValStateFile)
pv.Save()
logger.Info("Generated private validator", "keyFile", privValKeyFile,
"stateFile", privValStateFile)
}

nodeKeyFile := config.NodeKeyFile()
if tmos.FileExists(nodeKeyFile) {
logger.Info("Found node key", "path", nodeKeyFile)
} else {
if _, err := p2p.LoadOrGenNodeKey(nodeKeyFile); err != nil {
return err
}
logger.Info("Generated node key", "path", nodeKeyFile)
}

return nil
}
15 changes: 15 additions & 0 deletions gno.land/cmd/gnoland/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import (
"context"
"fmt"
"os"
)

func main() {
Copy link
Member

Choose a reason for hiding this comment

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

I believe it would be clearer to review the "main" and "newRootCmd" functions in the same file. I suggest switching back to "root.go" or renaming it to "main.go." However, it should remain as a single file, as it was previously.

rootCmd := newRootCmd()
if err := rootCmd.ParseAndRun(context.Background(), os.Args[1:]); err != nil {
_, _ = fmt.Fprintf(os.Stderr, "%+v\n", err)
os.Exit(1)
}
}
116 changes: 116 additions & 0 deletions gno.land/cmd/gnoland/mockio.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package main
Copy link
Member

Choose a reason for hiding this comment

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

See #1419


import (
"bytes"
"fmt"
"io"
"log"
"os"
)

// This is for testing purposes only.
// For mocking tests, we redirect os.Stdin so that we don't need to pass commands.IO,
// which includes os.Stdin, to all the server commands. Exposing os.Stdin in a blockchain node is not safe.
// This replaces the global variable and should not be used in concurrent tests. It's intended to simulate CLI input.
// We purposely avoid using a mutex to prevent giving the wrong impression that it's suitable for parallel tests.

type MockStdin struct {
origStdout *os.File
stdoutReader *os.File

outCh chan []byte

origStdin *os.File
stdinWriter *os.File
}

func NewMockStdin(input string) (*MockStdin, error) {
// Pipe for stdin. w ( stdinWriter ) -> r (stdin)
stdinReader, stdinWriter, err := os.Pipe()
if err != nil {
return nil, err
}

// Pipe for stdout. w( stdout ) -> r (stdoutReader)
stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil {
return nil, err
}

origStdin := os.Stdin
os.Stdin = stdinReader

_, err = stdinWriter.Write([]byte(input))
if err != nil {
stdinWriter.Close()
os.Stdin = origStdin
return nil, err
}

origStdout := os.Stdout
os.Stdout = stdoutWriter

outCh := make(chan []byte)

// This goroutine reads stdout into a buffer in the background.
go func() {
var b bytes.Buffer
if _, err := io.Copy(&b, stdoutReader); err != nil {
log.Println(err)
}
outCh <- b.Bytes()
}()

return &MockStdin{
origStdout: origStdout,
stdoutReader: stdoutReader,
outCh: outCh,
origStdin: origStdin,
stdinWriter: stdinWriter,
}, nil
}

// ReadAndRestore collects all captured stdout and returns it; it also restores
// os.Stdin and os.Stdout to their original values.
func (i *MockStdin) ReadAndClose() ([]byte, error) {
if i.stdoutReader == nil {
return nil, fmt.Errorf("ReadAndRestore from closed MockStdin %v", i)
}

// Close the writer side of the faked stdout pipe. This signals to the
// background goroutine that it should exit.
os.Stdout.Close()
out := <-i.outCh

os.Stdout = i.origStdout
os.Stdin = i.origStdin

if i.stdoutReader != nil {
i.stdoutReader.Close()
i.stdoutReader = nil
}

if i.stdinWriter != nil {
i.stdinWriter.Close()
i.stdinWriter = nil
}

return out, nil
}

// Call this in a defer function to restore and close os.Stdout and os.Stdin.
// This acts as a safeguard.
func (i *MockStdin) Close() {
os.Stdout = i.origStdout
os.Stdin = i.origStdin

if i.stdoutReader != nil {
i.stdoutReader.Close()
i.stdoutReader = nil
}

if i.stdinWriter != nil {
i.stdinWriter.Close()
i.stdinWriter = nil
}
}
36 changes: 36 additions & 0 deletions gno.land/cmd/gnoland/node_id.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package main

import (
"context"
"fmt"

"github.com/gnolang/gno/tm2/pkg/commands"
"github.com/gnolang/gno/tm2/pkg/p2p"
)

// Display a node's persistent peer ID to the standard output.
func newNodeIDCmd(bc baseCfg) *commands.Command {
Copy link
Member

Choose a reason for hiding this comment

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

By the way, the filename and function name should be consistent with "NodeID" and the subcommand should also be "node".

cmd := commands.NewCommand(
commands.Metadata{
Name: "node",
Copy link
Member

@moul moul Dec 7, 2023

Choose a reason for hiding this comment

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

As mentioned in the "validator" subcommand, I propose combining "node" and "validator" into one command, possibly called "info" or "node".


Edit: Upon further consideration, I believe it would be beneficial to implement a unique "info" command that provides the node ID, pubkey, and address.

Additionally, I propose using external helpers, potentially located in the contribs/ folder, to obtain more advanced information such as block height, connected peers, process status, and uptime. These helpers can primarily use the RPC info, read the config files or the raw datastore.

ShortUsage: "node",
ShortHelp: "display the node id for configuring persistent peers",
},
nil,
func(_ context.Context, args []string) error {
return execNodeID(bc)
},
)
return cmd
}

func execNodeID(bc baseCfg) error {
config := bc.tmConfig
nodeKey, err := p2p.LoadNodeKey(config.NodeKeyFile())
Copy link
Member

Choose a reason for hiding this comment

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

This is a more general question for everyone:
Why is the validator key != node key?

What is the point of keeping 2 separate keys? Why not just use the validator's private key as the node key, since they're both based on the 25519 curve?

if err != nil {
return err
}

fmt.Printf("NodeID: %v\n", nodeKey.ID())
return nil
}
Loading
Loading