Skip to content

Conversation

RazorClient
Copy link
Contributor

Allows ssz-serialization to be used both as a lib using nimble install and also as a sub-vendor in the nimbus build system

@RazorClient
Copy link
Contributor Author

Needs Hashtree-abi prysm to be added into the nim-lang/packages to work completely

@etan-status
Copy link
Contributor

Have published hashtree_abi: nim-lang/packages#3130

@etan-status
Copy link
Contributor

Can we test both of the flows in CI? The one from ../vendor, as well as the one via nimble? And also check that it is actually used on the platforms that support it.

@RazorClient
Copy link
Contributor Author

to do both checks will be possible i will change the ci

  • check that it is actually used on the platforms that support it.

this no idea

@etan-status
Copy link
Contributor

I think replacing const USE_HASHTREE_SHA256 = false in the case where hashtree is requested, supported by the cpu/toolchain, but not present in the buildsystem, with a static assert, would be good enough.

Or, adding a new test that checks that if PREFER_HASHTREE is set, and the cpu/toolchain supports it, that USE_HASHTREE is also true.

and yeah, could change the ci.yml, e.g., by deleting the vendor folder after running the tests, and then seeing if the tests still work (and still honor PREFER_HASHTREE)

@tersec
Copy link
Contributor

tersec commented Oct 7, 2025

https://github.com/status-im/nim-ssz-serialization/actions/runs/18290229123/job/52105696970?pr=123

[NimScript] exec: nim c  --styleCheck:usages --styleCheck:error --verbosity:0 --skipParentCfg --skipUserCfg --outdir:build '--nimcache:build/nimcache/$projectName'  --threads:on -d:PREFER_BLST_SHA256=false -d:PREFER_HASHTREE_SHA256=false --mm:refc -r tests/test_all
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization/digest.nim(38, 9) Hint: nimcrypto SHA256 backend enabled [User]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization/digest.nim(8, 52) Warning: imported and not used: 'importops' [UnusedImport]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization.nim(51, 77) Hint: 'writeFixedSized' cannot raise 'IOError' [XCannotRaiseY]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization/dynamic_navigator.nim(71, 39) Hint: 'fieldNavigatorImpl' cannot raise 'IOError' [XCannotRaiseY]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization/dynamic_navigator.nim(71, 39) Hint: 'fieldNavigatorImpl' cannot raise 'IOError' [XCannotRaiseY]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization/dynamic_navigator.nim(71, 39) Hint: 'fieldNavigatorImpl' cannot raise 'IOError' [XCannotRaiseY]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization/dynamic_navigator.nim(71, 39) Hint: 'fieldNavigatorImpl' cannot raise 'IOError' [XCannotRaiseY]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/ssz_serialization.nim(51, 77) Hint: 'writeFixedSized' cannot raise 'IOError' [XCannotRaiseY]
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/tests/test_ssz_union.nim(403, 1) template/generic instantiation of `suite` from here
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/tests/test_ssz_union.nim(404, 3) template/generic instantiation of `test` from here
/home/runner/.nimble/pkgs2/unittest2-0.2.4-746106a4dfefffce497f1693733f1c1513b5c62c/unittest2.nim(1139, 24) template/generic instantiation of `failingOnExceptions` from here
/home/runner/.nimble/pkgs2/unittest2-0.2.4-746106a4dfefffce497f1693733f1c1513b5c62c/unittest2.nim(1143, 26) template/generic instantiation of `failingOnExceptions` from here
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/tests/test_ssz_union.nim(409, 5) template/generic instantiation of `check` from here
/home/runner/work/nim-ssz-serialization/nim-ssz-serialization/nim/lib/system.nim(1806, 7) Error: parallel 'fields' iterator does not work for 'case' objects
stack trace: (most recent call last)

@RazorClient
Copy link
Contributor Author

@tersec i have moved the commit that caused that problem to a different branch.
That error in union is something i have already reported to @etan-status and he said he will be looking into it

@RazorClient
Copy link
Contributor Author

make an issue for the same
#125

@RazorClient
Copy link
Contributor Author

RazorClient commented Oct 7, 2025

added 2 new tests

      - name: Remove vendor and run tests with nimble
        run: |
          if [[ "${{ matrix.target.os }}" == "windows" ]]; then
            export NIMFLAGS="-d:nimRawSetjmp"
          fi
          echo "=== Removing vendor directory ==="
          rm -rf vendor
          echo "=== Testing with nimble (installed dependencies) ==="
          env TEST_LANG="c" NIMFLAGS="${NIMFLAGS} -d:PREFER_HASHTREE_SHA256" nimble test
          env TEST_LANG="cpp" NIMFLAGS="${NIMFLAGS} -d:PREFER_HASHTREE_SHA256" nimble test

also for

check that it is actually used on the platforms that support it

added/modified this part

const HASHTREE_SUPPORTED* = (defined(arm64) or defined(amd64)) and (
  (defined(linux) and defined(gcc)) or
  # llvm-mingw doesn't support hashtree well even with "-fno-integrated-as"
  # this is true with clang-17(19th June 2024).
  (defined(windows) and defined(gcc) and "clang" notin staticExec("gcc --version")) or
  (defined(linux) and defined(clang)) or
  (defined(macosx) and defined(clang) and defined(arm64))
)
when PREFER_HASHTREE_SHA256 and HASHTREE_SUPPORTED:
  {.hint: "Hashtree SHA256 backend enabled".}
  when tryImport ../vendor/hashtree/hashtree_abi:
    {.hint: "hashtree_abi found in vendor".}
    const HASHTREE_ABI_FOUND = true
  elif tryImport hashtree_abi:
    {.hint: "hashtree_abi package found".}
    const HASHTREE_ABI_FOUND = true
  else:
    {.hint: "hashtree_abi not found, disabling hashtree backend".}
    const HASHTREE_ABI_FOUND = false
else:
    const HASHTREE_ABI_FOUND = false

const USE_HASHTREE_SHA256* =
  PREFER_HASHTREE_SHA256 and HASHTREE_SUPPORTED and HASHTREE_ABI_FOUND

and added

  
  import unittest2
import ../ssz_serialization/digest

suite "Hashtree backend":

  test "preference + support implies USE == ABI_FOUND":
    when PREFER_HASHTREE_SHA256 and HASHTREE_SUPPORTED:
      check USE_HASHTREE_SHA256 == HASHTREE_ABI_FOUND

    test "constants are consistent":
      when USE_HASHTREE_SHA256:
        check PREFER_HASHTREE_SHA256
        check HASHTREE_SUPPORTED
        check HASHTREE_ABI_FOUND
        

any more cases to add here ??
cc @etan-status

@etan-status etan-status enabled auto-merge (squash) October 8, 2025 13:57
@etan-status etan-status disabled auto-merge October 8, 2025 14:01
@etan-status etan-status enabled auto-merge (squash) October 8, 2025 14:16
@etan-status etan-status merged commit 4d04d90 into status-im:master Oct 8, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants