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

[Core] StateHash Interfaces & Diagrams #252

Merged
merged 138 commits into from
Nov 28, 2022
Merged

Conversation

Olshansk
Copy link
Member

@Olshansk Olshansk commented Sep 27, 2022

Description

Interface updates and diagrams to document the cross-module flow for stateHash implementation.

Issue

Fixes part of #251

Type of change

Please mark the relevant option(s):

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Major breaking change
  • Documentation
  • Other

List of changes

The goal of this change is to show the interface changes and plan of action to implement the state hash implementation.

Documentation and external interface changes required for updating the state hash. Details related to the internals of the persistence module will be done in a follow up commit.

The major change is that the final quorumCertificate is only known at the time of committal, not proposal, so the corresponding changes were made as well as interface changes to make the terminology easier to follow with the corresponding documentation.

General

  • Update some comments / TODOs throughout the code
  • Handle errors where appropriate to support interface changes

UtilityContext interface changes

  • Change CommitPersistenceContext() to `Commit(quorumCert)
  • Change ReleaseContext() to Release() error

PostgresContext interface changes

  • Removed quorumCert from the SetProposalBlock method signature
  • Changed Commit() to Commit(quorumCert)
  • Renamed ResetContext to Release
  • Replace AppHash with UpdateAppHash
  • Added ReleaseWriteContext to the module level interface

Persistence Module/Interface changes

  • Reduce the cope of the StoreBlock function
  • Make storeBlock accept a quorumCert
  • Remove quorumCertificate from local state and corresponding setters/getters
  • Reduce the cope of the IndexTransactions function and remove from interface

Consensus module changes

  • Add error handling where appropriate corresponding to the interface changes

Testing

  • make develop_test
  • LocalNet w/ all of the steps outlined in the README

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have tested my changes using the available tooling
  • If applicable, I have made corresponding changes to related local or global README
  • If applicable, I have updated the corresponding CHANGELOG
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have added new diagrams using mermaid.js

deblasis and others added 14 commits November 15, 2022 14:50
## Description
This PR aims at introducing CLI commands relative to the Utility module: 

- Send
- Stake
- EditStake
- Unstake
- Unpause
- ChangeParameter

Consequently, it introduces an RPC server (HTTP) and the ability to "point" the CLI at specific nodes via flags 

Fixes [issue/112](#112)

## Type of change
Please mark the options that are relevant.

- [x] New feature (non-breaking change which adds functionality)

## How Has This Been Tested?

- [x] `make test_all`
- [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)

## Checklist
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have tested my changes using the available tooling
- [x] If applicable, I have made corresponding changes to related or global README
- [x] If applicable, I have added added new diagrams using [mermaid.js](https://mermaid-js.github.io)
- [x] If applicable, I have added tests that prove my fix is effective or that my feature works

* fix(config.proto): updated timeout type

* feat(RPC): basic RPC server with naive sync TX broadcast

* fix(RPC): fixed HTTP method for Health and Version routes

* chore(go.mod): tidy

* style(RPC): format

* chore(go.mod): tidy + vendor

* feat(Utility): CLI RPC client function

* feat(Utility): RPC defaults const

* feat(Utility): GetSignBytes

* feat(Utility): CLI utils

* feat(Utility): stake cmd scaffolding

* refactor(Utility): RPC server RoutesMap for CLI/client use

* refactor(Utility): RPC server exporting RouteKey and RoutesMap

for CLI/client use

* feat(Utility): CLI

calling updated QueryRPC to point at route from map

* refactor(Utility): RPC server exporting RouteKey and RoutesMap

for CLI/client use

* chore(Utility): Removed TODO

* fix(Utility): CLI: making use of the pwd flag

* fix(Utility): CLI: code review feedback

* fix(Utility): CLI custodial stake command: OutputAddr = fromAddr

* refactor(Utility): CLI: refactor command bindings

Also added missing functionality in commands other than Stake

* fix(Utility): Fix route after merge

* refactor(Utility): CLI: named return values fix

* test(Utility): CLI: simplified tests for PK parsing from file

* feat(Utility): RPC OpenApi spec and code generation

* refactor(Utility): RPC server refactoring using code generation

RPC server refactoring with code generation from openapi spec

* feat(Utility): RPC OpenApi spec and code generation

* feat(Utility): Updated RPC spec

* feat(Utility): Regenerated RPC boilerplate and updates

* docs(Utility): RPC and node docs barebones + RPC spec notes

* feat(Utility): Updated RPC spec

* docs(Utility): CLI docs barebones

* fix(Utility): CLI: added missing message handling for account

* fix(Shared): fixed RPC config

* feat(Utility): CLI updated to use the generated RPC client

* chore(go.mod): tidy

* refactor(Utility): CLI + RPC models in server.gen.go

* style(Utility): removed redundant struct definition

* docs(README.md): fixed LICENSE reference

* docs(README.md): updated with references to CLI, RPC and Node docs

* refactor(Shared): RPC config defaults -changing soon,I'm centralizin

* refactor(Shared): RPC config defaults -changing soon,I'm centralizin

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>

* fix(Utility): RPC: updated to use test_artifacts defaults

* refactor(Shared): RPC config defaults -changing soon,I'm centralizin

* feat(Utility): CLI: updated to use test_artifacts default

* docs(Utility): RPC and node basic docs

* docs(Utility): CLI: Changelog

* fix(Utility): CLI: fixed output to user. It shouldn't be logging

* fix(Utility): CLI code review feedback

* style(Utility): code review feedback

* feat(Tooling): Updated makefile commands to better handle codegen

* feat(Tooling): Updated makefile commands to better handle codegen

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>

* fix(Utility): Fix duplicated models

* chore(Utility): fixed versioning schema

* feat(Utility): CLI documentation generator + first stab at docs

* fix(Utility): CLI specific fixes

* fix(Utility): types fixes

* fix(Utility): RPC configs post-merge

* feat(Consensus): configOptions

* feat(P2P): configOptions

* feat(Utility): CLI: using configOptions to inject PK

* fix(Utility): RPC configs post-merge

* feat(Consensus): configOptions

* feat(P2P): configOptions

* fix(Utility): RPC fix post merge

* fix(Utility): RPC disabled by default because of TECHDEBT

* chore(go.mod): tidy

* fix(go.mod): tidy

* fix(CLI): test_artifacts

* fix(go.mod): tidy

* fix(CLI): updated to use new typesUtil.ActorType

* fix(CLI): runtime based init

* fix(test_artifacts): fix

* fix(CLI): runtime using WithRandomPK()

* fix(CLI): fixed client-only initialization

* fix(Makefile): protogen first

* feat(Utility): GetActorName function (can we autogenerate these?)

* fix(RPC): test_artifacts in runtime

* fix(go.mod): tidy

* fix(CLI): debug commands are now feature flagged

* chore(go.mod): tidy

* chore(CLI): git rm app/pocket/rpc/client.gen.go

* chore(CLI): s/j/tx and s/prepareTx/prepareTxJson

* refactor(shared): converters

* fix(CLI): debug nits

* refactor(CLI): confirmation and credentials

* chore(CLI): removed HACK todo

* fix(Makefile): premature protoc

* fix(RPC): added imports used in codegen files

* docs(RPC): added swagger editor link

* fix(RPC): added imports used in codegen files

* feat(RPC): config proto

* feat(RPC): RPCModule and noopRpcModule

* refactor(Shared): shared.Create -> shared.CreateNode

* refactor(RPC): moved scaffolding into rpc module

* fix(RPC): removed redundant file

* fix(CLI): debug merge fix

* docs(RPC): fixed link after refactoring

* docs(RPC): updated code organization post refactoring

* Update app/client/cli/utils.go

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* fix(RPC): gitignoring generated files

* style(Persistence): reverting space change

* refactor(Defaults): runtime/defaults package

* chore(CLI): issue handle

* chore(CLI): Issue #310 links

* refactor(CLI): preallocation fix

* style(CLI): oneMillion

* style(CLI): s/RelayChainIDs/relayChainIDs

* feat(Utility): ActorType.GetName()

* chore(Utility): tracking issue #142

* chore(Utility): GetBytes -> GetCanonicalBytes

* chore(CLI): actorCmd -> accountCmd

* docs(CLI): fromAddr note (address currently sourced from pk)

* chore(Runtime): new default value from main

* refactor(CLI): moving utility functions in utils.go

* chore(CLI): NewDebug -> NewDebugCommand

* fix(RPC): gitignoring generated files

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>

* refactor(Consensus): mocks with Return(nil) and not Do(...)

* docs(RPC): updated changelog versions

* fix(Makefile): generate_rpc_openapi

* fix(Makefile): fix for git clone + make develop_test

* Update rpc/v1/openapi.yaml

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* Update rpc/doc/CHANGELOG.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* Update rpc/noop_module.go

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* chore(Shared): added TODOes for ValidateXXX() in modules

* docs(RPC): broadcast_tx_sync summary updated

* Update rpc/doc/README.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* Update rpc/doc/README.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* Update rpc/doc/README.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* fix(Makefile): gitignoring generated files breaks tests fix

* docs(CLI): added todos for exactArgs

* chore(RPC): added types.go

* refactor(RPC): sourcing defaults from defaults not test_artifacts

* feat(CLI): system commands RPC🤝CLI

* feat(Consensus): CurrentRound() and CurrentStep()

* feat(RPC): /v1/consensus/round_state

* feat(RPC): /v1/consensus/round_state handler

* feat(RPC): /v1/consensus/round_state

* refactor(CLI): system command typoes copypastas

* feat(CLI): consensus commands -RoundState and individual ones

* chore(CLI): typo

* docs(CLI): short commands descriptions

* docs(Config): Adding some more color around configuration

* fix(CLI): tx message signing

* feat(CLI): reporting statuscode and body

* fix(Proto): deterministic

* refactor(CLI): prepareTxJSON -> prepareTxBytes

* docs(Docs): Adding some more color config + raw_hex_bytes

* refactor(CLI): Stake command

* fix(CLI): tx message signing

* feat(CLI): reporting statuscode and body

* Merge branch 'issue/utility_local_proof_of_stake_cli_CLI' into issue/utility_local_proof_of_stake_cli

* feat(Tooling): swagger-ui

* feat(RPC): cors feature flag

* Update utility/types/message.go

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* chore(Runtime): comment spacing

* docs(CLI): Changelog

* docs(CLI): changelog

* docs(RPC): changelog

* chore(Runtime): comment spacing

* Update runtime/docs/README.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* chore(Consensus): SetBus mock Do(func(modules.Bus) {}) -> Return()

* refactor(Consensus): mocks with Return(nil) and not Do(...)

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>

* chore(Consensus): SetBus mock Do(func(modules.Bus) {}) -> Return()

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>

* docs(Pocket): changelog

* Update app/pocket/doc/README.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* Update app/pocket/doc/README.md

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* docs(RPC): changelog

* docs(RPC): docs TOC

* docs(RPC): Transports -> Endpoints

* feat(Tooling): swagger-ui

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>

* docs(RPC): swagger ui link to editor and ref to make cmd

* feat(rpcServer): rpcServer is now IntegratableModule

* chore(Shared): tracking TODO (implement validations) #334

* fix(RPC): merge fix

* chore(go.mod): tidy

* chore(go.mod): tidy

* feat(Tooling): added empty mock_module package for cold start

* docs(RPC): nit

* Update app/client/cli/consensus.go

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>

* docs(CLI): better commands descriptions

* feat(CLI): boldText helper

* style(CLI): nit: real estate

* refactor(CLI): ConsensusState

* docs(CLI): updated docs

* docs(CLI): \n at the end of sentences in Stake command desc.

* docs(CLI): regenerated docs

* fix(RPC): int64 on RoundState fields

* refactor(Shared): unexporting XXModuleName

* feat(node): single source of truth for version + overridable

Signed-off-by: Alessandro De Blasis <alex@deblasis.net>
Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>
…Module (#320)

Reducing the scope of public functions in the persistence package from public to private for readability purposes
@Olshansk Olshansk requested review from jessicadaugherty and andrewnguyen22 and removed request for jessicadaugherty November 16, 2022 00:52
C[Am I the leader?] --> |Yes| D
C[Am I the leader?] --> |No| Z

D[Did I get any prepareQCs?] --> |Find highest valid PrepareQC| E
Copy link
Contributor

Choose a reason for hiding this comment

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

If not, let's include it in your chart

@Olshansk Olshansk requested review from andrewnguyen22 and removed request for iajrz November 22, 2022 17:19
@Olshansk
Copy link
Member Author

@andrewnguyen22 Updated.

@deblasis Would appreciate your feedback on the interface as well.

Copy link
Contributor

@deblasis deblasis 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 good, just a couple of comments/questions

shared/docs/PROTOCOL_STATE_HASH.md Outdated Show resolved Hide resolved
Comment on lines +132 to +136
GetBlockHashAtHeight(height int64) ([]byte, error)
GetBlocksPerSession(height int64) (int, error)
GetLatestProposerAddr() []byte
GetLatestBlockProtoBytes() []byte
GetLatestBlockHash() string
GetLatestBlockTxs() [][]byte
GetProposerAddr() []byte
GetBlockProtoBytes() []byte
GetBlockHash() string
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why GetBlockHashAtHeight returns []byte and GetBlockHash returns string?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a result of all the string / byte inconsistencies we have throughout the codebase.

In #284, I consolidate it to: GetBlockHash(height int64) ([]byte, error)

@Olshansk Olshansk requested a review from deblasis November 28, 2022 17:50
Copy link
Contributor

@deblasis deblasis left a comment

Choose a reason for hiding this comment

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

LGTM

@Olshansk Olshansk merged commit daaec74 into main Nov 28, 2022
@Olshansk Olshansk deleted the issues/251/statehash_interface branch November 28, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus Consensus specific changes core Core infrastructure - protocol related utility Utility specific changes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants