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: claim/proof messages, relayer, & genesis #24

Merged
merged 140 commits into from
Sep 27, 2023
Merged

feat: claim/proof messages, relayer, & genesis #24

merged 140 commits into from
Sep 27, 2023

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Sep 18, 2023

Changes

Dependencies

  • Updated smt version in go.mod

Until pokt-network/smt#19 and pokt-network/smt#18 are merged, the go.mod needs to use a replace to point to a temporary version (poktroll-needs branch) of the smt package which includes ~~support for serialization of SparseMerkelTrees as well as implementation of the KV store and #ProveClosest().

On-chain servicer logic (module)

  • Generated Claim and Proof messages
  • Switched applicable fields' types from string to []byte due to ignite limitations (see: supported types)

Off-chain servicer logic (aka relayer)

  • Added relayer subcommand (i.e. poktrolld relayer -h) to isolate client-side / off-chain servicer logic
  • Relayer proxy component is proxying requests to/from the client and the service (relay-chain)
  • Session tracker component is using the ServiceClient to listen (via websocket) for new blocks

Genesis

  • Added localnet_regenesis make target which regenerates the localnet genesis.json, node_key.json, priv_validator_key.json, & test keyring
  • Uses ignite chain init (see: docs) to populate genesis & keyring according to config.yml (see: ignite docs)
  • All accounts have static mnemonics to minimize version control churn (i.e. generating all new keys ea. regenesis)

Running

Start celestia and the sequencer:

make localnet_down
rm -rf ~/.poktroll  # remove any existing local keyring
ignite chain init   # regenerate local genesis, keyring, & configs from config.yml
make localnet_up

Stake app1 & servicer1

make app1_stake && make servicer1_stake

Start the relayer:

poktrolld relayer \
--node tcp://localhost:36657 \
--signing-key servicer1 \
--keyring-backend test

Start anvil (mock relay-chain node):
(see: anvil docs)

anvil -p 8547 -b 5

Request a relay via cast (on the default port: 8545):
(see: cast docs)

cast block

Subscribe for new blocks:

wscat --connect ws://localhost:8546
Connected (press CTRL+C to quit)
> {"id":1,"jsonrpc":"2.0","method":"eth_subscribe","params":["newHeads"]}

Query for servicer1 claims:

poktrolld query servicer claims $(poktrolld keys show servicer1 -a --keyring-backend test)

red-0ne and others added 6 commits September 15, 2023 15:13
  * Implemented a servicer client
  * Relayminer aka Off-chain servicer as poktrolld sub-command
  * Generated Claim and Proof messages
- use `bytes` for binary data
- update CLI usages to use hex-encoded input args
* pokt/main:
  chore: remove .pb.go & .pb.gw.go files from version control (already gitignored)
bryanchriswhite and others added 5 commits September 19, 2023 09:25
- move signal handling to cmd code
- use cancellable context
- add waitgroup to track goroutines
* pokt/relayer-with-ws:
  WIP: proxy sign replies
@bryanchriswhite bryanchriswhite changed the title feat: Add relayminer & claim/proof msgs to servicer module feat: claim/proof messages & relayer Sep 19, 2023
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

Still making my way through by just leaving a few comments.
Screenshot 2023-09-27 at 2 47 25 PM


.PHONY: localnet_regenesis
localnet_regenesis:
# NOTE: intentionally not using --home <dir> flag to avoid overwriting the test keyring
Copy link
Member

Choose a reason for hiding this comment

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

Consider using --home=$(POKTROLLD_HOME) like we do in other places

"pub_key": null,
"account_number": "1",
"sequence": "0"
},
Copy link
Member

Choose a reason for hiding this comment

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

I've not looked into ignite accounts yet but ignite chain init will generate all the accounts and genesis to match all the accounts in config.yml. It seems more straightforward and conventional to me to use it for this purpose.

Ccing @okdas who will likely be looking closer at best practices here.

Also, I was under the impression that after poktrolld add-genesis-account one would also have to run poktrolld gentx followed by poktrolld collect-gentxs.

My gut says no (an account is just a balance) but I haven't looked into it.


option go_package = "poktroll/x/servicer/types";

message EventClaim {
Copy link
Member

Choose a reason for hiding this comment

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

  1. Can you leave a comment on what/how/where this is used?
  2. Can you link me to where this naming convention / pattern is the best practice?

keyName string
// wsURL is the URL of the websocket endpoint to connect to for RPC
// service over websocket transport (with /subscribe support).
wsURL string
Copy link
Member

Choose a reason for hiding this comment

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

Is this a cometbft endpoint? It's unclear from the comment as to who is providing this endpoint.

// getNextRequestId increments and returns the JSON-RPC request ID which should
// be used for the next request. These IDs are expected to be unique (per request).
func (client *servicerClient) getNextRequestId() uint64 {
client.nextRequestId++
Copy link
Member

Choose a reason for hiding this comment

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

Two comments:

  1. Does it have to be monotonically increasing or will random work as well? Jargon: entropy, distributed tracing, etc...

  2. Do you think we need a lock around this to where we defer the unlock?

Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

Overall, pretty good and clean. A few things for the main repo:

  1. Need more comments related to the structs and the fields
  2. Will need to spend more time on the types (protos & structs). We should:
    1. Look at what was done in v0 (e.g. their version of the session manager)
    2. Look at what we designed in v1 (e.g. the relay types)
  3. Need a diagram showing the relationship in the components under relayer
  4. The most confusing part right now is definitely the proxy and the session manager. Will take closer looks at those when we review them in smaller pieces.

type sessionWithTree struct {
sessionInfo *types.Session
tree *smt.SMST
treeStore smt.KVStore
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to keep references to the store, the path AND the tree?


func (s *sessionWithTree) SessionTree() *smt.SMST {
// if the tree is closed, we need to re-open it from disk
if s.closed {
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused on when it's okay or not okay to reopen a tree. My assumption is that if it's closed, it's closed forever.

Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

Works!

Screenshot 2023-09-27 at 7 01 07 PM

@Olshansk Olshansk merged commit df351a6 into main Sep 27, 2023
@Olshansk Olshansk deleted the relayminer branch September 27, 2023 23:08
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

4 participants