[Test] Unit Tests to validate Block Size increases + minor improvements

Olshansk commented 1 year ago

Description

Add unit tests to verify that block size modifications work as expected

Fixes

Validation of the ability to increase block size to increase the throughput of the network.

For reference, the Tendermint block size is hardcoded to have a cap of 100MB; source.

Changes

Primary Changes

Improved TestBlockSize_ChangeParamValue to validate that the block size parameter is updated and set correctly
Added TestBlockSize_ChangeAndFillBlockSize to verify that the block size parameter works as intended

Secondary Changes

Added a global variable for modifying tendermint’s timeout commt in app/common_test;
Fixed the block size being exported by the blockchain state
Renamed and reformatted a few functions so they are easier to read and understand

Tertiary Changes

Updated the v1/query/upgrade endpoint to return a JSON string
Added a few helpful comments throughout the code
Automatic VSCode formatting changes

Testing

See the comments in the code for complexity related to testing TestBlockSize_MaximumSize.

As seen below:

The number of transactions included in the block gradually increases as the block size increases
When the block size exceeds 64000, the txs in the backlog are in the block
The block is then big enough to include all txs every time, so we are at a "steady state" of ~49

Executed with: go test -count=1 -v -run TestBlockSize_ChangeAndFillBlockSize ./app

The output of the test is as follows:

    tx_test.go:1107: Transactions accumulated at height=5: totalTxBytes=1019, len(res.Txs)=5, TotalTxCount=5, blockSize=2000
    tx_test.go:1107: Transactions accumulated at height=6: totalTxBytes=1017, len(res.Txs)=5, TotalTxCount=5, blockSize=2000
    tx_test.go:1107: Transactions accumulated at height=7: totalTxBytes=1017, len(res.Txs)=5, TotalTxCount=5, blockSize=2000
    tx_test.go:1107: Transactions accumulated at height=8: totalTxBytes=1029, len(res.Txs)=5, TotalTxCount=5, blockSize=4000
    tx_test.go:1107: Transactions accumulated at height=9: totalTxBytes=3065, len(res.Txs)=15, TotalTxCount=15, blockSize=8000
    tx_test.go:1107: Transactions accumulated at height=10: totalTxBytes=6931, len(res.Txs)=34, TotalTxCount=34, blockSize=16000
    tx_test.go:1107: Transactions accumulated at height=11: totalTxBytes=14873, len(res.Txs)=73, TotalTxCount=73, blockSize=64000
    tx_test.go:1107: Transactions accumulated at height=12: totalTxBytes=24030, len(res.Txs)=118, TotalTxCount=118, blockSize=256000
    tx_test.go:1107: Transactions accumulated at height=13: totalTxBytes=9775, len(res.Txs)=48, TotalTxCount=48, blockSize=512000
    tx_test.go:1107: Transactions accumulated at height=14: totalTxBytes=9987, len(res.Txs)=49, TotalTxCount=49, blockSize=1024000
    tx_test.go:1107: Transactions accumulated at height=15: totalTxBytes=9986, len(res.Txs)=49, TotalTxCount=49, blockSize=2048000
    tx_test.go:1107: Transactions accumulated at height=16: totalTxBytes=9983, len(res.Txs)=49, TotalTxCount=49, blockSize=4096000
    tx_test.go:1107: Transactions accumulated at height=17: totalTxBytes=9983, len(res.Txs)=49, TotalTxCount=49, blockSize=8192000
    tx_test.go:1107: Transactions accumulated at height=18: totalTxBytes=9984, len(res.Txs)=49, TotalTxCount=49, blockSize=16384000
    tx_test.go:1107: Transactions accumulated at height=19: totalTxBytes=9778, len(res.Txs)=48, TotalTxCount=48, blockSize=32768000

Next Steps

[ ] Two DISCUSS items are left for the reviewer in the PR
This also, in a separate PR, will be validated end-to-end on LocalNet before pushing to TestNet

cr-gpt[bot] commented 1 year ago

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables for this repo. you could follow readme for more information

reviewpad[bot] commented 1 year ago

AI-Generated Pull Request Summary: This pull request includes changes across multiple files, focusing on improving code clarity, validation, error handling, and streamlining functions. Key updates include reorganizing import orders, adding new test functions, modifying existing tests, and implementing additional comments for clarification. The EndBlock function in the Manager struct has been refactored to simplify updating consensus parameters, and several functions have been changed to methods of the AppModule struct. New fields like BlockByteSize have been incorporated into DefaultParams() and relevant test functions. Logging in the types/param.go file has been enhanced, and various changes have been made to app/query.go. Changes in other files include refactoring, updating values, and adding new context creation in functions like app/pocket.go. Overall, these changes aim to enhance readability, maintainability, and functionality in the codebase.

cr-gpt[bot] commented 1 year ago

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables for this repo. you could follow readme for more information

reviewpad[bot] commented 1 year ago

AI-Generated Pull Request Summary: This pull request includes several improvements and changes across multiple files in terms of code organization, readability, maintainability, and functionality. Notable updates include the addition of the BlockByteSize parameter and corresponding functions in various files, refactoring of the ActivateAdditionalParameters method and ConsensusParamUpdate function, reorganization of import statements and comments, adjustments to error handling and logging, and updates to test files. Additionally, various formatting changes have been made to improve clarity and adhere to Go documentation guidelines. Overall, the functional behavior of the code is mostly left unchanged, with improvements primarily focused on code organization, readability, and maintainability.

reviewpad[bot] commented 1 year ago

AI-Generated Pull Request Summary: This pull request includes changes across multiple files related to block sizes, consensus configuration, governance parameters, tests, and code formatting. Key updates include adding new tests, refactoring existing tests, updating function signatures and methods, improving error handling and logging, and adjusting context versions and parameters. The changes are intended to enhance code readability, maintainability, and overall functionality, while also ensuring consistency in settings related to Tendermint nodes during testing.

Olshansk commented 1 year ago

@PoktBlade PTAL when you have time, I think this one should be okay to merge in.

nodiesBlade commented 1 year ago

LGTM, look forward to testing this out in testnet 🚀

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request includes various changes across multiple files, mainly focused on improving code organization, readability, and maintainability, as well as enhancing test coverage. Some notable modifications include:

Updating functions and methods related to block size handling.
Adding field 'BlockByteSize' to various structs and functions.
Renaming and encapsulating functions (e.g., ActivateAdditionalParameters to activateAdditionalParameters and making it a method of the AppModule structure).
Refactoring and enhancing test functions related to block size, governance parameters, and protocol upgrades.
Reformatting and updating comments for improved clarity and consistency.
Rearranging import statements and updating import order in several files.

Overall, these changes aim to improve the quality and functionality of the codebase.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request includes changes across multiple files, focusing on the following aspects:

Rearranging import statements and code organization.
Improving error handling, logging, and code readability.
Updating function signatures and converting some standalone functions to methods.
Introducing new variables and parameters, such as BlockByteSize and tendermintTimeoutCommit.
Modifying various functions and tests related to block size and change parameters functionality.
Updating comments for better clarification and understanding of the code.

Overall, these changes aim to enhance the stability, functionality, and readability of the codebase.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request includes a variety of changes across multiple files. Key updates include adding a new parameter BlockByteSize, making changes to functions and methods, updating comments for clarity, and refactoring code. Additional changes involve modifying test cases and configuration values, improving code organization and readability, and introducing new log messages for better insight into processes.

Some important modifications include the refactoring of the ActivateAdditionalParameters function, the addition of new tests related to changing block size parameters through governance, and updates to the x/pocketcore/types/params.go file. The changes aim to improve overall code quality, readability, and maintainability while providing better insight into processes and ensuring consistency in naming conventions and functionality.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request introduces a new function called BlockByteSize and updates various files to accommodate this new block byte size parameter. The codebase has been updated with improved error handling, logging, testing, and refactoring. Major changes include the addition of new import statements, the modification of existing methods, and the removal of unnecessary code. Overall, this PR enhances the functionality, readability, and maintainability of the existing code.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request includes various enhancements, refactors, and added test cases. Key changes include refactoring the ActivateAdditionalParameters function, adding a new BlockByteSize function, parameter, and default value, modifying comments, reordering import statements, and updating the handling of consensus updates in EndBlock. Additionally, changes have been made to the ConsensusParamUpdate function to improve readability, and tests have been added to the x/gov/module.go file. The changes are focused on improving code readability, maintainability, and testing the functionality of changing block size parameters.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request includes numerous changes across multiple files, mostly focusing on improving code readability, test coverage, and refactoring for better maintainability. Key updates include the introduction of a BlockByteSize field and related tests, updating comments and import statements, and refactoring functions related to consensus updates and parameter activation. Additionally, Tendermint node configurations have been modified to use constants, and new functions and methods have been added to handle block size parameters and additional parameter activations. Error handling and logging were also improved in specific cases. Overall, these changes contribute to a more robust and clean codebase.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request includes changes to several files, primarily focusing on refactoring functions, updating test cases, and improving code readability. Major changes consist of refactoring ActivateAdditionalParameters function in x/pocketcore/module.go, adding new test functions in app/tx_test.go, renaming and refactoring functions in x/gov/module.go, adding log messages and comments in types/param.go, introducing a new function BlockByteSize in x/pocketcore/keeper/params.go, modifying the TestMode variable in codec.go, updating comments and minor refactoring in various files, and modifying functions related to block size parameter updating. Additionally, import statements have been reorganized, new comments have been added for better understanding, and code has been reformatted for increased readability. Overall, this pull request improves code organization, enhances functionality, and ensures better code quality.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request introduces various changes across multiple files, focusing mainly on improving code readability, functionality, and test cases. The significant changes include:

Modifications to imports, constants, structs, and methods in several files.
Refactoring and restructuring of various methods and functions for better readability and structure.
Addition of new fields and functions to handle parameters, such as BlockByteSize.
Updates and enhancements to test cases, including new test cases for validating parameter functionality and lifecycle.
Improved commenting and explanations for methods, functions, and variables.

Notable changes are made to app/pocket.go, app/query.go, pocketcore/keeper/keeper.go, and other related files. Overall, the update aims to improve code quality, maintainability, and testing capabilities.

reviewpad[bot] commented 1 year ago

AI-Generated Summary: This pull request introduces various updates across multiple files focusing on improving code organization, readability, and providing additional context to functions and parameters. Notable changes include adding a new parameter BlockByteSize to the TestDefaultParams function, refactoring the ConsensusParamUpdate function to use a new helper function consensusBlockSizeParamUpdate, updating the default genesis state for the pocketcore module, and enhancing testing scenarios related to governance parameters and block size changes. Multiple files also have adjusted import statements, updated comments, and better error handling. Overall, these changes enhance readability, simplify code, and provide better maintainability of the codebase.

Olshansk commented 1 year ago

Going to merge this in. I reviewed the CircleCI build and confirmed none of the failures are related to this changes and are being handled in another PR at this point.

pokt-network / pocket-core