[TECHDEBT][Persistence] Fix Persistence Module TODOs - Lowercase Functions #149

[jessicadaugherty]

Objective

A general cleanup issue is needed to tackle TODO's and ensure the persistence module is usable/readable by making functions lowercase as part of #172.

Origin Document

Should follow issue-#128, issue-#105, issue-#147 and issue-#148 the persistence module is messier and more difficult to understand than the developers would want for organic external contribution.

Goals / Deliverables

• Make functions lower case (i.e. not exposed outside the specific package) if it does not need to be. This is a best-effort task.

General issue checklist

• Update the appropriate CHANGELOG
• Update the README
• If applicable, update the source code tree explanation
• If applicable, add or update a state, sequence or flowchart diagram using mermaid
• Update any relevant global documentation & references
• Document small issues / TODOs along the way

---

Creator: @andrewnguyen22
Co-creator: @Olshansk

[Olshansk]

A good starting point is to use regex find inside the persistence module for func (.*) [A-Z] and comparing it to the interface in modules/persistence_module.go to determine what MUST be exposed.

[Olshansk]

@Jasonyou1995 Please see the comment I left here: #320 (comment)

Let me know if you need any further clarification or help on finding functions that should be lowercased (i.e. not exposed publically).

[Jasonyou1995]

I created a BASH script check_lowercase_function.sh

#!/bin/bash

# Run this function with ./pocket/ in the same folder as this script (making it executable)
# Usage: $ ./check_lowercase_function.sh
# Mainly check result in:
#   ./report/5_output_function_usage_count_outside_persistence

# Creating a new `report` repo for result saving
if [[ ! -e report ]]; then
    mkdir report
fi

POKT="./pocket/"
POKT_PERSISTENCE="./pocket/persistence/"

egrep -rI "^func( )?\(.*\)( )?[A-Z].*" "${POKT_PERSISTENCE}" > ./report/1_raw_capitalized_func

sed -E 's/^.* ([A-Z][A-Za-z]*)\(.*/\1/' ./report/1_raw_capitalized_func > ./report/2_function_names


# Finding out the out of persistence usage (zero usage can be lowercased)
cat ./report/2_function_names | while read line; do
    echo "[${line}]" >> ./report/3_detailed_function_usage_outside_persistence
    grep -IR --exclude-dir="*persistence*" "${line}" "${POKT}" >> ./report/3_detailed_function_usage_outside_persistence
    echo >> ./report/3_detailed_function_usage_outside_persistence
done

cat ./report/2_function_names | while read line; do
    grep -IR --exclude-dir="*persistence*" "${line}" "${POKT}" | wc -l >> ./report/4_usage_count_for_each_pub_func_outside_persistence
done


paste -d " " \
    ./report/2_function_names \
    ./report/4_usage_count_for_each_pub_func_outside_persistence \
    > ./report/5_output_function_usage_count_outside_persistence

# Print output
echo "[Functions with zero usage outside ${POKT_PERSISTENCE}]"
echo
awk '{ if ($2 == 0) print }' ./report/5_output_function_usage_count_outside_persistence

This script will print out all the Public (uppercase) functions that were never used outside the persistence module/repo.

[Jasonyou1995]

Here are the outputs for functions with zero usage outside ./pocket/persistence/:

GetTableName        0
GetChainsTableName        0
GetActorSpecificColName        0
GetTableSchema        0
GetChainsTableSchema        0
GetQuery        0
GetAllQuery        0
GetExistsQuery        0
GetReadyToUnstakeQuery        0
GetOutputAddressQuery        0
GetStakeAmountQuery        0
GetPausedHeightQuery        0
GetUnstakingHeightQuery        0
GetChainsQuery        0
InsertQuery        0
UpdateQuery        0
UpdateChainsQuery        0
UpdateUnstakingHeightQuery        0
UpdatePausedHeightQuery        0
UpdateUnstakedHeightIfPausedBeforeQuery        0
SetStakeAmountQuery        0
ClearAllQuery        0
ClearAllChainsQuery        0
GetPools        0
GetAccounts        0
GetApplications        0
GetParams        0
GetPools        0
GetAccounts        0
GetApplications        0
GetParams        0
InsertQuery        0
UpdateChainsQuery        0
GetChainsTableSchema        0
GetChainsQuery        0
ClearAllChainsQuery        0
GetActorFromRow        0
GetChainsForActor        0
InsertActor        0
UpdateActor        0
GetActorsReadyToUnstake        0
SetActorUnstakingHeightAndStatus        0
GetActorPauseHeightIfExists        0
SetActorStatusAndUnstakingHeightIfPausedBefore        0
SetActorStakeAmount        0
GetActorStakeAmount        0
GetCtxAndTx        0
GetCtx        0
DebugClearAll        0
GetValidatorStakedTokens        0

[Olshansk]

@Jasonyou1995 Pro tip

If you add bash after the 3 backticks, it'll add syntax highlighting in place:

We don't have a place for scripts like this at the moment, so let's keep it here and figure out if we could/should productionize if we see the need for it to come up in the future.

[Jasonyou1995]

@Jasonyou1995 Pro tip

If you add bash after the 3 backticks, it'll add syntax highlighting in place:

We don't have a place for scripts like this at the moment, so let's keep it here and figure out if we could/should productionize if we see the need for it to come up in the future.

Sounds great! Thanks for the tips. I will modify the script a little bit to correct the package matching issue (I didn't know that Go treat sub folders as different packages).

[Olshansk]

Highly recommend taking this Go course in our learning guide: https://github.com/pokt-network/pocket/blob/main/docs/learning/README.md#golang

[Jasonyou1995]

Highly recommend taking this Go course in our learning guide: https://github.com/pokt-network/pocket/blob/main/docs/learning/README.md#golang

I will spend sometime to get these basics done ASAP!

[Jasonyou1995]

Here is the updated script and list of functions only used in the Persistence module.

Script

#!/bin/bash

# Author: Jason You
# Email: [email protected]
# Last modified: 10/31/2022
#
# Note: This script might take a few minutes to run for large projects


# Run this function with ./pocket/ in the same folder as this script
#
#   Usage: $ ./check_lowercase_function.sh
#
# Result stored in
#
#   ./report/3_potential_lower_case_functions
#
###########


# Creating a new `report` repo for result saving
if [[ ! -e report ]]; then
    mkdir report
fi

# Set the folders that contains functions we are checking
POKT="./pocket"
POKT_PERSISTENCE="./pocket/persistence"

# Finding all public functions in given persistence folder (not subfolders)
find "${POKT_PERSISTENCE}" -type f -exec egrep -I "^func( )?\(.*\)( )?[A-Z].*" {} \; > ./report/1_raw_capitalized_func

# Extracting function names and store them
sed -E 's/^.* ([A-Z][A-Za-z]*)\(.*/\1/' ./report/1_raw_capitalized_func > ./report/2_function_names


# Finding out the out of persistence usage (zero usage can be lowercased)
cat ./report/2_function_names | while read line; do
    # number of time this function used outside the package (-w: full word)
    usage_count_outside=`grep -IRw --exclude-dir="${POKT_PERSISTENCE}" "${line}" "${POKT}" | wc -l`

    # skip this function when there are out of package usage
    if [ $usage_count_outside -gt 0 ]; then
        continue
    fi

    # search for usage in the sub-directories of ./pocket/persistence/
    usage_count_inside=`find "${POKT_PERSISTENCE}" -type d | tail -n +2 | xargs grep -IRw "${line}" | wc -l`

    # skip this function when there are out of package usage
    if [ $usage_count_inside -gt 0 ]; then
        continue
    fi

    # Add the function that have no usage outside the package
    echo "${line}" >> ./report/3_potential_lower_case_functions
done

# Output functions that has no out of package usage
echo "[Functions with zero usage outside ${POKT_PERSISTENCE}]"
echo
cat ./report/3_potential_lower_case_functions

Output (List of functions can be private)

GetActor
GetActorFromRow
GetChainsForActor
SetActorStakeAmount
GetActorStakeAmount
GetCtxAndTx
GetCtx
SetValidatorStakedTokens
GetValidatorStakedTokens

[Jasonyou1995]

The test_all failed, so I might need to spend some time double check the cause before merge.

2022/11/02 22:32:16 About to close context at height -1.
2022/11/02 22:32:16 Populating genesis state...
2022/11/02 22:32:16 Populating genesis state...
2022/11/02 22:32:16 About to commit context at height 0.
2022/11/02 22:32:16 Starting persistence module...
2022/11/02 22:32:16 About to release context at height 0.
FAIL
FAIL    github.com/pokt-network/pocket/utility/test     42.790s
ok      github.com/pokt-network/pocket/utility/types    0.232s
FAIL
make: *** [test_all] Error 1