Merge #2877: [Tests][Scripts] Bump Python version to 3.8 and add more…

… linting

c340533 Bump python version to 3.8 (Fuzzbawls)
17bc5bf tests: remove redundant function (Fuzzbawls)
ec22835 scripted-diff: test: Use py3.5 bytes::hex() method (Fuzzbawls)
139ae9d lint: more python linting (Fuzzbawls)
c757ce3 scripts: clean github-merge.py and symbol-check.py (Fuzzbawls)
6f822ae scripts: additional E275 fixups (Fuzzbawls)
07d1c56 scripted-diff: test: Remove brackets after assert (Fuzzbawls)
ef83f5b tests: Avoid using mutable default parameter values (Fuzzbawls)
77b42a2 Lint: enable mypy linting for python files (Fuzzbawls)
19862fd scripts: exclude wallet_bumpfee.py from linting (Fuzzbawls)
8c0dd38 scripts: remove outdated python3 workaround in scrypt.py (Fuzzbawls)
5619891 scripts: remove NONFATAL from security-check.py (fanquake)
457f2a8 scripts: no-longer check for 32 bit windows in security-check.py (fanquake)
036ab88 tests: add type hints to the test framework's script.py (Fuzzbawls)
a560910 Strictly enforce instance attrs in critical functional test classes. (Justin Turner Arthur)
f46217b test: Remove python3.4 workaround in feature_dbcrash (MarcoFalke)

Pull request description:

  This PR offers a bit of an overhaul to our python test suite and python standards. The high level information is that we:
  - migrate from the EOL'd Python 3.5 to a more recent version (3.8)
  - strip out some ancient workaround code that is no longer necessary
  - add new linters that will provide an early warning if new python code is not up to standards

  The bulk of the changes here are via scripted-diff commits, or adapted cherry-picks from upstream.

ACKs for top commit:
  Liquid369:
    ACK c340533
  panleone:
    re tACK c340533

Tree-SHA512: 12ec442232fa0cc40b377844105792144862943ead7a0803754bf36eb4def6307aeb91753c4c6e386f7610cf9b5a76e82ac3b982a4e24be56468b8707135b49e

.github/workflows/build-and-test.yml

@@ -19,14 +19,15 @@ jobs:
       - name: Initialize Python
         uses: actions/setup-python@v4
         with:
-          python-version: 3.6
+          python-version: 3.8
 
       - name: Install Dependencies
         run: |
           python -m pip install --upgrade pip
-          pip install codespell==1.13.0
-          pip install flake8==3.8.3
-          pip install vulture==0.29
+          pip install codespell==2.2.5
+          pip install flake8==5.0.4
+          pip install mypy==1.4.1
+          pip install vulture==2.6
           pip install yq
 
           curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/

.python-version

@@ -1 +1 @@
-3.5.6
+3.8.16

configure.ac

@@ -82,8 +82,8 @@ AC_PATH_TOOL(RANLIB, ranlib)
 AC_PATH_TOOL(STRIP, strip)
 AC_PATH_TOOL(GCOV, gcov)
 AC_PATH_PROG(LCOV, lcov)
-dnl Python 3.5 is specified in .python-version and should be used if available, see doc/dependencies.md
-AC_PATH_PROGS([PYTHON], [python3.5 python3.6 python3.7 python3.8 python3 python])
+dnl Python 3.8 is specified in .python-version and should be used if available, see doc/dependencies.md
+AC_PATH_PROGS([PYTHON], [python3.8 python3.9 python3.10 python3.11 python3.12 python3 python])
 AC_PATH_PROG(GENHTML, genhtml)
 AC_PATH_PROG([GIT], [git])
 AC_PATH_PROG(RUSTC, rustc)

contrib/devtools/security-check.py

@@ -15,7 +15,6 @@
 READELF_CMD = os.getenv('READELF', '/usr/bin/readelf')
 OBJDUMP_CMD = os.getenv('OBJDUMP', '/usr/bin/objdump')
 OTOOL_CMD = os.getenv('OTOOL', '/usr/bin/otool')
-NONFATAL = {} # checks which are non-fatal for now but only generate a warning
 
 def check_ELF_PIE(executable):
     '''
@@ -116,51 +115,38 @@ def check_ELF_Canary(executable):
             ok = True
     return ok
 
-def get_PE_dll_characteristics(executable):
-    '''
-    Get PE DllCharacteristics bits.
-    Returns a tuple (arch,bits) where arch is 'i386:x86-64' or 'i386'
-    and bits is the DllCharacteristics value.
-    '''
+def get_PE_dll_characteristics(executable) -> int:
+    '''Get PE DllCharacteristics bits'''
     p = subprocess.Popen([OBJDUMP_CMD, '-x',  executable], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True)
     (stdout, stderr) = p.communicate()
     if p.returncode:
         raise IOError('Error opening file')
-    arch = ''
     bits = 0
     for line in stdout.splitlines():
         tokens = line.split()
-        if len(tokens)>=2 and tokens[0] == 'architecture:':
-            arch = tokens[1].rstrip(',')
         if len(tokens)>=2 and tokens[0] == 'DllCharacteristics':
             bits = int(tokens[1],16)
-    return (arch,bits)
+    return bits
 
 IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA = 0x0020
 IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE    = 0x0040
 IMAGE_DLL_CHARACTERISTICS_NX_COMPAT       = 0x0100
 
 def check_PE_DYNAMIC_BASE(executable):
     '''PIE: DllCharacteristics bit 0x40 signifies dynamicbase (ASLR)'''
-    (arch,bits) = get_PE_dll_characteristics(executable)
-    reqbits = IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE
-    return (bits & reqbits) == reqbits
+    bits = get_PE_dll_characteristics(executable)
+    return (bits & IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE) == IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE
 
-# On 64 bit, must support high-entropy 64-bit address space layout randomization in addition to DYNAMIC_BASE
-# to have secure ASLR.
+# Must support high-entropy 64-bit address space layout randomization
+# in addition to DYNAMIC_BASE to have secure ASLR.
 def check_PE_HIGH_ENTROPY_VA(executable):
     '''PIE: DllCharacteristics bit 0x20 signifies high-entropy ASLR'''
-    (arch,bits) = get_PE_dll_characteristics(executable)
-    if arch == 'i386:x86-64':
-        reqbits = IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA
-    else: # Unnecessary on 32-bit
-        assert(arch == 'i386')
-        reqbits = 0
-    return (bits & reqbits) == reqbits
+    bits = get_PE_dll_characteristics(executable)
+    return (bits & IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA) == IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA
 
 def check_PE_NX(executable):
     '''NX: DllCharacteristics bit 0x100 signifies nxcompat (DEP)'''
-    (arch,bits) = get_PE_dll_characteristics(executable)
+    bits = get_PE_dll_characteristics(executable)
     return (bits & IMAGE_DLL_CHARACTERISTICS_NX_COMPAT) == IMAGE_DLL_CHARACTERISTICS_NX_COMPAT
 
 def get_MACHO_executable_flags(executable):
@@ -237,18 +223,12 @@ def identify_executable(executable):
                 continue
 
             failed = []
-            warning = []
             for (name, func) in CHECKS[etype]:
                 if not func(filename):
-                    if name in NONFATAL:
-                        warning.append(name)
-                    else:
-                        failed.append(name)
+                    failed.append(name)
             if failed:
                 print('%s: failed %s' % (filename, ' '.join(failed)))
                 retval = 1
-            if warning:
-                print('%s: warning %s' % (filename, ' '.join(warning)))
         except IOError:
             print('%s: cannot open' % filename)
             retval = 1