update for latest Optional spec

[etan-status]

EIP-6475 was bumped to emit an additional 0x01 byte in the Some case of optionals to allow nesting Optional and Optional[List] and to provide compatibility with Union serialization. Note that the None case is still serialized as empty.

https://eips.ethereum.org/EIPS/eip-6475
ethereum/EIPs#6945

[tersec]

Apropos 5d3eac5 see also df1d03c

[tersec]

And compare 824ba45 and ecf42b0

At some some of these fixes should make it in, regardless of the PRs carrying them, at some point, so people stop having to rediscover/reinvent them

[etan-status]

OK, I have cherry-picked the explanatory comments that you added there.

[etan-status]

In hindsight, cleaner if we simply merge your PR first. @tersec

[tersec]

In hindsight, cleaner if we simply merge your PR first. @tersec

done

[tersec]

It is interesting if the macOS timeout is a related issue to the failure in the other platforms. Either points to something salient:

• if it's a root-cause-same-issue, which is manifesting in a way which is a bit more unambiguously wrong (SIGSEGV can mean many things, but looping infinitely like that points to the possible underlying failure mode being within a smaller feasible set which does not include many usual causes of SIGSEGV; or

• it's an entirely different issue and something in the relevant code here tickles Nim devel in a way that makes it behave just generally unpredictably, which means that this starts tipping into unknown/unknown territories of exposure to scope of Nim bugs.

[etan-status]

I tried to build your minimal repro on macos to see if it exhibits the same behaviour, but I can't even build NIM_COMMIT=devel locally:

cmd: bin/nim c --noNimblePath --skipUserCfg --skipParentCfg --hints:off --warnings:off --hints:off koch
/Users/etan/Documents/Repos/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system.nim(411, 17) template/generic instantiation from here
/Users/etan/Documents/Repos/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system/inclrtl.nim(50, 21) Error: invalid pragma: sinkInference: on

Indeed, interesting, where the different behaviour for i386 Linux (works fine), macos C (timeout), and others (sigsegv) is coming from. 1-3 bugs (or possibly more).

As long as it is just devel that's failing, I'm not sure how much time we should invest into debugging this, as this seems to be a Nim issue for an untagged development version. The other possibility is an issue with faststreams (@zah). If we can rule out that one, your minimal repro shared on Discord is likely very useful for reporting (thanks again for creating that one).

[tersec]

Yeah, agree it's not worth that much more effort as long as it's devel only. One thing to watch for will be is if patches from devel are merged into version-2-0 which introduce whatever's happening in devel to 2.0. If that happens, it'll be worth looking at more closely.

[etan-status]

On version-2-0, same issues as on devel, minus the oddities regarding i386 working and macos C timing out.

Locally, same build issue as on devel.

make update V=1 NIM_COMMIT=version-2-0

cmd: bin/nim c --noNimblePath --skipUserCfg --skipParentCfg --hints:off --warnings:off --hints:off koch
/Users/etan/Documents/Repos/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system.nim(411, 17) template/generic instantiation from here
/Users/etan/Documents/Repos/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system/inclrtl.nim(50, 21) Error: invalid pragma: sinkInference: on
make[2]: *** [build-nim] Error 1
make[1]: *** [vendor/nimbus-build-system/vendor/Nim/bin/nim] Error 2

[tersec]

Interesting, well, at least a build error is more dispositively correct or not.

[tersec]

Any idea why in the CI, it hadn't seemed as necessary to prevent version-2-0 from being tested on ORC on non-i386 platforms?

[etan-status]

Back when the split into --gc:refc / --gc:orc builds was introduced in PR #41, the --gc:orc build was only enabled on devel, as version-2-0 was not yet being tested.

When PR #43 introduced version-2-0 testing, the --gc:orc build was still just testing against devel, so it was never run against version-2-0 until this PR where I noticed just before the absence of --gc:orc testing for version-2-0, and added this now with the latest checks.

Behaviour is also broken with ORC, but compared to devel, i386 also fails and macos C no longer times out

[etan-status]

Note that the new logic in ci.yml was updated to be more resilient against such bitrot, by inverting the condition so that if a new version is added, both --mm:refc and mm:orc are tested.

[tersec]

i386 also fails and macos C no longer times out

Interesting that both changes are temporally correlated, but with opposite failure-valences.

[etan-status]

Yeah, I guess that Nim's own CI has more time-granular results for what commits changed the behaviour in nim-ssz-serialization compilation. As they don't build using our ci.yml, they should have run into this issue sooner than we did.

[tersec]

I found that Nim commit 2dcfd732609a2cfa805e5a94cc105399a2f18632 defaults to refc, so if one runs its default mode, does not end in SIGSEGV on amd64 Linux, but if one uses --mm:orc does, so at least parts of this predate ORC being default.

Thus, probably both version-2-0 and current devel branches inherited the same issues before/when they started diverging.

Edit: yeah, version-1-6 with --mm:orc does the same thing for me, it's just not tested/relevant for Status. But this is not a new issue, it's just newly exposed by ORC becoming default in version-2-0 and devel.

[etan-status]

CI is now green, with these caveats:

• On version-2-0, tests fail with --mm:orc
• On devel:
  • On i386 (32-bit Linux), tests succeed
  • On macOS, tests get stuck and time out when compiling with --mm:orc to C
  • On macOS, tests fail with --mm:orc when targeting CPP
  • On other platforms (64-bit Linux and Windows), tests fail with --mm:orc

These caveats have been enforced in ci.yml so that we see when Nim behaviour changes.