Skip to content

Conversation

pmikolajczyk41
Copy link
Member

Problem

json:"...,omitempty"  doesn't work for golang arrays if they don't have zero length - if a field is an array type and not slice then it's always included in the marshaled json string. Same for structs, aliases etc.

Why it's bad: it's easy to break backward compatibility or even introduce diversion, e.g. if you add optional field to params.ChainConfig.ArbitrumChainParams of type common.Address that happens to be type Address [AddressLength]byte, the genesis block state will diverge - when initializing ArbOS, chain config is marshaled to json and saved to arbos state.

Solution

The only existing linter for this was: https://github.com/andydotdev/omitlint. However:

  • it doesn't seem to be maintained anymore
  • no stars, usages, mentions in Google browse
  • not compatible with https://golangci-lint.run/docs/plugins/module-plugins/
  • installing it as a separate binary adds to repo requirement + might not be that safe
  • simple enough to adapt and reimplement quickly

So I simply added a new linter to our in-house set. Fortunately, it seems that our codebase is not affected by an incorrect usage of omitempty.

Copy link

github-actions bot commented Oct 15, 2025

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
2141 3 2138 0
View the top 3 failed tests by shortest run time
TestVersion40
Stack Traces | 5.110s run time
... [CONTENT TRUNCATED: Keeping last 20 lines]
=== CONT  TestVersion40
    precompile_inclusion_test.go:90: goroutine 454872 [running]:
        runtime/debug.Stack()
        	/opt/hostedtoolcache/go/1.25.3/x64/src/runtime/debug/stack.go:26 +0x5e
        github.com/offchainlabs/nitro/util/testhelpers.RequireImpl({0x40824d0, 0xc076687340}, {0x4040700, 0xc152bfabd0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc076687340, {0x4040700, 0xc152bfabd0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1723 +0x5d
        github.com/offchainlabs/nitro/system_tests.testPrecompiles(0xc076687340, 0x28, {0xc083b65df8, 0x5, 0x39?})
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:90 +0x371
        github.com/offchainlabs/nitro/system_tests.TestVersion40(0xc076687340?)
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:71 +0x64b
        testing.tRunner(0xc076687340, 0x3ccaa48)
        	/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1997 +0x465
        
    precompile_inclusion_test.go:90: �[31;1m [] execution aborted (timeout = 5s) �[0;0m
�[90mposted new batch 50�[0;0m
--- FAIL: TestVersion40 (5.11s)
TestEthSyncing
Stack Traces | 22.430s run time
... [CONTENT TRUNCATED: Keeping last 20 lines]
DEBUG[10-16|09:58:14.051] Journaled pathdb diff layer              root=bd9dba..0235b5 parent=157621..d69964 id=67                 block=66
INFO [10-16|09:58:14.052] Persisted dirty state to disk            size=291.24KiB elapsed=5.730ms
INFO [10-16|09:58:14.052] Blockchain stopped
TRACE[10-16|09:58:14.052] P2P networking is spinning down
INFO [10-16|09:58:14.053] New Key                                  name=Faucet            Address=0xaF24Ca6c2831f4d4F629418b50C227DF0885613A
INFO [10-16|09:58:14.053] New Key                                  name=RollupOwner       Address=0x57Ff0F473737a1c161bfF9efDF016F7991585088
INFO [10-16|09:58:14.053] New Key                                  name=Sequencer         Address=0xb386a74Dcab67b66F8AC07B4f08365d37495Dd23
INFO [10-16|09:58:14.054] New Key                                  name=Validator         Address=0x83FFCFaCE2Fb0E1286686815503608A16EF41e47
INFO [10-16|09:58:14.054] New Key                                  name=User              Address=0x7E23C8862920797d81916d62c274dd9217113e28
INFO [10-16|09:58:14.054] Allocated trie memory caches             clean=154.00MiB dirty=256.00MiB
INFO [10-16|09:58:14.054] State schema set to default              scheme=path
WARN [10-16|09:58:14.055] Head block is not reachable
DEBUG[10-16|09:58:14.055] Current full block not old enough to freeze err="freezing threshold is not available"
INFO [10-16|09:58:14.055] Initialising Ethereum protocol           network=412,346 dbversion=<nil>
INFO [10-16|09:58:14.055] Load database journal from disk
INFO [10-16|09:58:14.056] State snapshot generator is not found
INFO [10-16|09:58:14.056] Starting snapshot generation             root=56e81f..63b421 accounts=0  slots=0     storage=0.00B   dangling=0 elapsed="3.066µs"
INFO [10-16|09:58:14.056] Initialized path database                triecache=154.00MiB statecache=102.00MiB buffer=256.00MiB state-history="last 90000 blocks"
INFO [10-16|09:58:14.056] Writing custom genesis block
--- FAIL: TestEthSyncing (22.43s)
TestTimeboostTxsTimeoutByBlock
Stack Traces | 41.110s run time
... [CONTENT TRUNCATED: Keeping last 20 lines]
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=16  size=3.29KiB   time="42.289µs"  gcnodes=938  gcsize=181.14KiB  gctime=2.551876ms  livenodes=145   livesize=28.88KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=24  size=4.59KiB   time="68.648µs"  gcnodes=962  gcsize=185.73KiB  gctime=2.620314ms  livenodes=121   livesize=24.29KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=18  size=3.56KiB   time="50.915µs"  gcnodes=980  gcsize=189.29KiB  gctime=2.671059ms  livenodes=103   livesize=20.73KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=18  size=3.53KiB   time="74.138µs"  gcnodes=998  gcsize=192.82KiB  gctime=2.744826ms  livenodes=85    livesize=17.20KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=17  size=3.40KiB   time="43.531µs"  gcnodes=1015 gcsize=196.22KiB  gctime=2.787806ms  livenodes=68    livesize=13.80KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=17  size=3.52KiB   time="45.174µs"  gcnodes=1032 gcsize=199.74KiB  gctime=2.83282ms   livenodes=51    livesize=10.28KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=16  size=3.24KiB   time="41.107µs"  gcnodes=1048 gcsize=202.98KiB  gctime=2.873786ms  livenodes=35    livesize=7.04KiB
DEBUG[10-16|10:00:05.464] Dereferenced trie from memory database   nodes=19  size=3.63KiB   time="46.788µs"  gcnodes=1067 gcsize=206.61KiB  gctime=2.920464ms  livenodes=16    livesize=3.41KiB
DEBUG[10-16|10:00:05.465] Dereferenced trie from memory database   nodes=16  size=3.41KiB   time="37.17µs"   gcnodes=1083 gcsize=210.02KiB  gctime=2.957514ms  livenodes=0     livesize=0.00B
DEBUG[10-16|10:00:05.465] Dereferenced trie from memory database   nodes=0   size=0.00B     time=171ns       gcnodes=1083 gcsize=210.02KiB  gctime=2.957604ms  livenodes=0     livesize=0.00B
DEBUG[10-16|10:00:05.465] Dereferenced trie from memory database   nodes=0   size=0.00B     time=211ns       gcnodes=1083 gcsize=210.02KiB  gctime=2.957705ms  livenodes=0     livesize=0.00B
INFO [10-16|10:00:05.465] Blockchain stopped
DEBUG[10-16|10:00:05.467] redis producer: check responses starting
DEBUG[10-16|10:00:05.467] checkResponses                           responded=0 errored=0 checked=0
TRACE[10-16|10:00:05.467] Refreshing our lock                      id=auctioneer-64dff666-1760608765320082670
TRACE[10-16|10:00:05.469] P2P networking is spinning down
INFO [10-16|10:00:05.471] Context done while waiting redis streams to be ready, failed to start
ERROR[10-16|10:00:05.471] Context closed, autonomous auctioneer shutting down
ERROR[10-16|10:00:05.472] Context closed, autonomous auctioneer shutting down
--- FAIL: TestTimeboostTxsTimeoutByBlock (41.11s)

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@eljobe eljobe assigned eljobe and unassigned magicxyyz Oct 16, 2025
@eljobe eljobe enabled auto-merge October 16, 2025 09:41
@eljobe eljobe added this pull request to the merge queue Oct 16, 2025
auto-merge was automatically disabled October 16, 2025 10:05

Pull Request is not mergeable

Merged via the queue into master with commit 4941f52 Oct 16, 2025
23 checks passed
@eljobe eljobe deleted the pmikolajczyk/nit-3992-omitempty branch October 16, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants