Skip to content

Commit

Permalink
[Testing] refractor: param update E2E tests as integration tests (#826)
Browse files Browse the repository at this point in the history
## Summary

Ports the `MsgUpdateParams` and `MsgUpdateParam` E2E tests to
integration tests, improving execution speed and maintainability.

## Depends on

- #827 
- #820 
- #821
- #815

## Dependents

- #809 

## Issue

- #799

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [x] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [ ] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [x] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [x] I create and reference any new tickets, if applicable
- [x] I have left TODOs throughout the codebase, if applicable

---------

Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: red-0ne <[email protected]>
  • Loading branch information
3 people authored Sep 25, 2024
1 parent e87e7da commit 2a9d91b
Show file tree
Hide file tree
Showing 17 changed files with 983 additions and 252 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ docusaurus_start: check_npm check_node ## Start the Docusaurus server

.PHONY: docs_update_gov_params_page
docs_update_gov_params_page: ## Update the page in Docusaurus documenting all the governance parameters
go run tools/scripts/generate_docs_params.go
go run tools/scripts/docusaurus/generate_docs_params.go

######################
### Ignite Helpers ###
Expand Down
81 changes: 3 additions & 78 deletions docusaurus/docs/develop/developer_guide/adding_params.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,88 +56,13 @@ message Params {
}
```

### 2 Update the Parameter E2E Tests

Update the E2E test files (e.g., `update_params.feature` and `update_params_test.go`)
to include scenarios that test the new parameter.

#### 2.1 Scenario Example

```gherkin
# NB: If you are reading this and the proof module has parameters
# that are not being updated in this test, please update the test.
Scenario: An authorized user updates all "proof" module params
Given the user has the pocketd binary installed
And all "proof" module params are set to their default values
And an authz grant from the "gov" "module" account to the "pnf" "user" account for the "/poktroll.proof.MsgUpdateParams" message exists
When the "pnf" account sends an authz exec message to update all "proof" module params
| name | value | type |
| new_parameter_name | 100 | int64 |
Then all "proof" module params should be updated
```

#### 2.2 Scenario Outline Example

```gherkin
# NB: If you are reading this and any module has parameters that
# are not being updated in this test, please update the test.
Scenario Outline: An authorized user updates individual <module> module params
Given the user has the pocketd binary installed
And all "<module>" module params are set to their default values
And an authz grant from the "gov" "module" account to the "pnf" "user" account for the "<message_type>" message exists
When the "pnf" account sends an authz exec message to update "<module>" the module param
| name | value | type |
| <param_name> | <param_value> | <param_type> |
Then the "<module>" module param "<param_name>" should be updated
Examples:
| module | message_type | param_name | param_value | param_type |
| proof | /poktroll.proof.MsgUpdateParam | new_parameter_name | 100 | int64 |
```

#### 2.3 Step Definition Helpers Example

The related changes to the step definition, presented below via an example,
should be made in `e2e/tests/parse_params_test.go`.

```go
func (s *suite) newProofMsgUpdateParams(params paramsMap) cosmostypes.Msg {
msgUpdateParams := &prooftypes.MsgUpdateParam{
Params: &prooftypes.Params{},
}

for paramName, paramValue := range params {
switch paramName {
case prooftypes.ParamNewParameterName:
msgUpdateParams.Params.NewParameterName = uint64(paramValue.value.(int64))
default:
s.Fatalf("unexpected %q type param name %q", paramValue.typeStr, paramName)
}
}
### 2 Update the Parameter Integration Tests

return msgUpdateParams
}
```

#### 2.4 Update switch statement to support new param

The related changes to the step definition, presented below via an example,
should be made in `e2e/tests/parse_params_test.go`.

```go
case prooftypes.ModuleName:
params := prooftypes.DefaultParams()
paramsMap := s.expectedModuleParams[moduleName]

newParameter, ok := paramsMap[prooftypes.ParamNewParameterName]
if ok {
params.NewParameter = uint64(newParameter.value.(int64))
}
```
// TODO_DOCUMENT(@bryanchriswhite, #826)

### 3. Update the Default Parameter Values

In the corresponding Go file (e.g., `params.go`), define the default value, key, and parameter name for the
In the corresponding Go file (e.g., `x/<module>/types/params.go`), define the default value, key, and parameter name for the
new parameter and include the default in the `NewParams` and `DefaultParams` functions.

```go
Expand Down
6 changes: 6 additions & 0 deletions docusaurus/docs/develop/developer_guide/test_suites.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
sidebar_position: 5
title: Test Suites
---

// TODO_DOCUMENT(@bryanchriswhite)
15 changes: 10 additions & 5 deletions docusaurus/docs/protocol/governance/params.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ DO NOT EDIT: this file was generated by make docs_update_gov_params_page
- [Adding a new parameter](#adding-a-new-parameter)
- [Parameters](#parameters)

## Access Control

// TODO_DOCUMENT(@bryanchriswhite) tl;dr, authz.

## Updating governance parameter values

// TODO_DOCUMENT(@bryanchriswhite)

## Updating this page

```
Expand All @@ -27,12 +35,9 @@ Please follow the instructions in [this guide](../../develop/developer_guide/add


|Module | Field Type | Field Name |Comment |
|-------|------------|------------|--------|
| `application` | `uint64 ` | `max_delegated_gateways` | max_delegated_gateways defines the maximum number of gateways that a single application can delegate to. This is used to prevent performance issues in case the relay ring signature becomes too large. |
|-------|------------|------------|--------|| `application` | `uint64 ` | `max_delegated_gateways` | max_delegated_gateways defines the maximum number of gateways that a single application can delegate to. This is used to prevent performance issues in case the relay ring signature becomes too large. |
| `proof ` | `bytes ` | `relay_difficulty_target_hash` | TODO_FOLLOWUP(@olshansk, #690): Either delete this or change it to be named "minimum" relay_difficulty_target_hash is the maximum value a relay hash must be less than to be volume/reward applicable. |
| `proof ` | `float ` | `proof_request_probability` | proof_request_probability is the probability of a session requiring a proof if it's cost (i.e. compute unit consumption) is below the ProofRequirementThreshold. |
| `proof ` | `uint64 ` | `proof_requirement_threshold` | proof_requirement_threshold is the session cost (i.e. compute unit consumption) threshold which asserts that a session MUST have a corresponding proof when its cost is equal to or above the threshold. This is in contrast to the this requirement being determined probabilistically via ProofRequestProbability. TODO_MAINNET: Consider renaming this to `proof_requirement_threshold_compute_units`. |
| `service ` | `uint64 ` | `add_service_fee` | The amount of uPOKT required to add a new service. This will be deducted from the signer's account balance, and transferred to the pocket network foundation. |
| `shared ` | `uint64 ` | `num_blocks_per_session` | num_blocks_per_session is the number of blocks between the session start & end heights. |
| `shared ` | `uint64 ` | `grace_period_end_offset_blocks` | grace_period_end_offset_blocks is the number of blocks, after the session end height, during which the supplier can still service payable relays. Suppliers will need to recreate a claim for the previous session (if already created) to get paid for the additional relays. |
| `shared ` | `uint64 ` | `claim_window_open_offset_blocks` | claim_window_open_offset_blocks is the number of blocks after the session grace period height, at which the claim window opens. |
Expand All @@ -41,4 +46,4 @@ Please follow the instructions in [this guide](../../develop/developer_guide/add
| `shared ` | `uint64 ` | `proof_window_close_offset_blocks` | proof_window_close_offset_blocks is the number of blocks after the proof window open height, at which the proof window closes. |
| `shared ` | `uint64 ` | `supplier_unbonding_period_sessions` | supplier_unbonding_period_sessions is the number of sessions that a supplier must wait after unstaking before their staked assets are moved to their account balance. On-chain business logic requires, and ensures, that the corresponding block count of the unbonding period will exceed the end of any active claim & proof lifecycles. |
| `shared ` | `uint64 ` | `application_unbonding_period_sessions` | application_unbonding_period_sessions is the number of sessions that an application must wait after unstaking before their staked assets are moved to their account balance. On-chain business logic requires, and ensures, that the corresponding block count of the application unbonding period will exceed the end of its corresponding proof window close height. |
| `tokenomics` | `uint64 ` | `compute_units_to_tokens_multiplier` | The amount of upokt that a compute unit should translate to when settling a session. |
| `shared ` | `uint64 ` | `compute_units_to_tokens_multiplier` | The amount of upokt that a compute unit should translate to when settling a session. DEV_NOTE: This used to be under x/tokenomics but has been moved here to avoid cyclic dependencies. |
4 changes: 4 additions & 0 deletions docusaurus/docs/protocol/upgrades/module_params.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ Pocket Network utilizes an off-chain governance mechanism that enables the commu
- [Examples](#examples)
- [Block Size Change](#block-size-change)

## Access Control

// TODO_DOCUMENT(@bryanchriswhite) tl;dr, authz.

## Examples

### Block Size Change
Expand Down
16 changes: 8 additions & 8 deletions docusaurus/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1810,10 +1810,15 @@
dependencies:
"@types/mdx" "^2.0.0"

"@node-rs/jieba-darwin-arm64@1.10.0":
"@node-rs/jieba-linux-x64-gnu@1.10.0":
version "1.10.0"
resolved "https://registry.npmjs.org/@node-rs/jieba-darwin-arm64/-/jieba-darwin-arm64-1.10.0.tgz"
integrity sha512-IhR5r+XxFcfhVsF93zQ3uCJy8ndotRntXzoW/JCyKqOahUo/ITQRT6vTKHKMyD9xNmjl222OZonBSo2+mlI2fQ==
resolved "https://registry.npmjs.org/@node-rs/jieba-linux-x64-gnu/-/jieba-linux-x64-gnu-1.10.0.tgz"
integrity sha512-rS5Shs8JITxJjFIjoIZ5a9O+GO21TJgKu03g2qwFE3QaN5ZOvXtz+/AqqyfT4GmmMhCujD83AGqfOGXDmItF9w==

"@node-rs/[email protected]":
version "1.10.0"
resolved "https://registry.npmjs.org/@node-rs/jieba-linux-x64-musl/-/jieba-linux-x64-musl-1.10.0.tgz"
integrity sha512-BvSiF2rR8Birh2oEVHcYwq0WGC1cegkEdddWsPrrSmpKmukJE2zyjcxaOOggq2apb8fIRsjyeeUh6X3R5AgjvA==

"@node-rs/jieba@^1.6.0":
version "1.10.0"
Expand Down Expand Up @@ -4614,11 +4619,6 @@ fs.realpath@^1.0.0:
resolved "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz"
integrity sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==

fsevents@~2.3.2:
version "2.3.3"
resolved "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz"
integrity sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==

function-bind@^1.1.2:
version "1.1.2"
resolved "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz"
Expand Down
25 changes: 2 additions & 23 deletions e2e/tests/stake_supplier_steps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ package e2e

import (
"reflect"
"strings"
"unicode"

"github.com/stretchr/testify/require"

"github.com/pokt-network/poktroll/testutil/cases"
sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
)

Expand Down Expand Up @@ -61,7 +60,7 @@ func paramsAnyMapFromParamsStruct(paramStruct any) paramsAnyMap {
for i := 0; i < paramsReflectValue.NumField(); i++ {
fieldValue := paramsReflectValue.Field(i)
fieldStruct := paramsReflectValue.Type().Field(i)
paramName := toSnakeCase(fieldStruct.Name)
paramName := cases.ToSnakeCase(fieldStruct.Name)

fieldTypeName := fieldStruct.Type.Name()
// TODO_IMPROVE: MsgUpdateParam currently only supports int64 and not uint64 value types.
Expand All @@ -78,23 +77,3 @@ func paramsAnyMapFromParamsStruct(paramStruct any) paramsAnyMap {
}
return paramsMap
}

func toSnakeCase(str string) string {
var result strings.Builder

for i, runeValue := range str {
if unicode.IsUpper(runeValue) {
// If it's not the first letter, add an underscore
if i > 0 {
result.WriteRune('_')
}
// Convert to lowercase
result.WriteRune(unicode.ToLower(runeValue))
} else {
// Otherwise, just append the rune as-is
result.WriteRune(runeValue)
}
}

return result.String()
}
95 changes: 0 additions & 95 deletions e2e/tests/update_params.feature

This file was deleted.

Loading

0 comments on commit 2a9d91b

Please sign in to comment.