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

[Olshansk]

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

[nodiesBlade]

For first round, LGTM. I ran it locally and got succeeding tests as well.

I don't see the initial dangers of increasing the block size from the code perspective, but at the same time, my tendermint knowledge is not as vast as I'd like. A lot of increasing block size implications has to deal with gossiping and consensus, so in the testnet, we should ensure that we have a distributed set of validators from multiple node providers (my suggestion at least 5 different parties).

--- PASS: TestBlockSize_ChangeParams (5.80s)
=== RUN   TestBlockSize_MaximumSize
    tx_test.go:1110: Transactions accumulated at height=5: totalTxBytes=1018, len(res.Txs)=5, TotalTxCount=5, blockSize=2000
    tx_test.go:1110: Transactions accumulated at height=6: totalTxBytes=1018, len(res.Txs)=5, TotalTxCount=5, blockSize=2000
    tx_test.go:1110: Transactions accumulated at height=7: totalTxBytes=1017, len(res.Txs)=5, TotalTxCount=5, blockSize=2000
    tx_test.go:1110: Transactions accumulated at height=8: totalTxBytes=1028, len(res.Txs)=5, TotalTxCount=5, blockSize=4000
    tx_test.go:1110: Transactions accumulated at height=9: totalTxBytes=3063, len(res.Txs)=15, TotalTxCount=15, blockSize=8000
    tx_test.go:1110: Transactions accumulated at height=10: totalTxBytes=6930, len(res.Txs)=34, TotalTxCount=34, blockSize=16000
    tx_test.go:1110: Transactions accumulated at height=11: totalTxBytes=14894, len(res.Txs)=73, TotalTxCount=73, blockSize=128000
    tx_test.go:1110: Transactions accumulated at height=12: totalTxBytes=14254, len(res.Txs)=70, TotalTxCount=70, blockSize=256000
    tx_test.go:1110: Transactions accumulated at height=13: totalTxBytes=7127, len(res.Txs)=35, TotalTxCount=35, blockSize=512000
    tx_test.go:1110: Transactions accumulated at height=14: totalTxBytes=7338, len(res.Txs)=36, TotalTxCount=36, blockSize=1024000
    tx_test.go:1110: Transactions accumulated at height=15: totalTxBytes=7138, len(res.Txs)=35, TotalTxCount=35, blockSize=2048000
    tx_test.go:1110: Transactions accumulated at height=16: totalTxBytes=7143, len(res.Txs)=35, TotalTxCount=35, blockSize=4096000
    tx_test.go:1110: Transactions accumulated at height=17: totalTxBytes=7537, len(res.Txs)=37, TotalTxCount=37, blockSize=8192000
    tx_test.go:1110: Transactions accumulated at height=18: totalTxBytes=7335, len(res.Txs)=36, TotalTxCount=36, blockSize=16384000
    tx_test.go:1110: Transactions accumulated at height=19: totalTxBytes=7344, len(res.Txs)=36, TotalTxCount=36, blockSize=32768000
--- PASS: TestBlockSize_MaximumSize (98.64s)

[cr-gpt]

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]

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.

[reviewpad]

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]

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

[nodiesBlade]

LGTM, look forward to testing this out in testnet 🚀

[reviewpad]

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]

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

1. Rearranging import statements and code organization.
2. Improving error handling, logging, and code readability.
3. Updating function signatures and converting some standalone functions to methods.
4. Introducing new variables and parameters, such as BlockByteSize and tendermintTimeoutCommit.
5. Modifying various functions and tests related to block size and change parameters functionality.
6. 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]

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]

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]

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]

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]

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]

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:

1. Modifications to imports, constants, structs, and methods in several files.
2. Refactoring and restructuring of various methods and functions for better readability and structure.
3. Addition of new fields and functions to handle parameters, such as BlockByteSize.
4. Updates and enhancements to test cases, including new test cases for validating parameter functionality and lifecycle.
5. 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]

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]

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.