celestia-node
celestia-node copied to clipboard
enable govet fieldalignment
digging through old issues found the request for integrating "structslop" with ci. The tool of which does some struct memory alignment checks. I was surprised this wasn't already present in golangci-lint and voila they do have some of these checks as part of govet
- this enabled the
fieldalignmentgovet check - at current run, we have a handlful (30 or so) suggestions, which i think is quite noisy but yes, probably decent optimizations
@Wondertan i think this satisfies the original request in the old issue, if you want to proceed, maybe we impart the fixes as part of this PR too
fixes #893
current warnings
header/header.go:36:21: fieldalignment: struct with 360 pointer bytes could be 344 (govet)
type ExtendedHeader struct {
^
header/header.go:241:23: fieldalignment: struct with 56 pointer bytes could be 40 (govet)
return json.Marshal(&struct {
^
header/header.go:256:10: fieldalignment: struct with 56 pointer bytes could be 40 (govet)
aux := &struct {
^
nodebuilder/node/admin.go:15:13: fieldalignment: struct with 24 pointer bytes could be 16 (govet)
type module struct {
^
nodebuilder/node/admin.go:29:11: fieldalignment: struct with 16 pointer bytes could be 8 (govet)
type Info struct {
^
nodebuilder/fraud/lifecycle.go:23:53: fieldalignment: struct with 88 pointer bytes could be 80 (govet)
type ServiceBreaker[S service, H libhead.Header[H]] struct {
^
share/p2p/params.go:11:17: fieldalignment: struct with 40 pointer bytes could be 8 (govet)
type Parameters struct {
^
api/rpc/server.go:22:13: fieldalignment: struct with 56 pointer bytes could be 48 (govet)
type Server struct {
^
libs/edssser/edssser.go:20:13: fieldalignment: struct with 32 pointer bytes could be 8 (govet)
type Config struct {
^
libs/edssser/edssser.go:30:14: fieldalignment: struct with 104 pointer bytes could be 64 (govet)
type EDSsser struct {
^
nodebuilder/blob/cmd/blob.go:162:15: fieldalignment: struct with 16 pointer bytes could be 8 (govet)
response := struct {
^
nodebuilder/blob/cmd/blob.go:205:16: fieldalignment: struct with 56 pointer bytes could be 48 (govet)
type tempBlob struct {
^
nodebuilder/p2p/cmd/p2p.go:274:11: fieldalignment: struct with 40 pointer bytes could be 24 (govet)
return struct {
^
nodebuilder/p2p/cmd/p2p.go:313:11: fieldalignment: struct with 40 pointer bytes could be 24 (govet)
return struct {
^
nodebuilder/p2p/cmd/p2p.go:381:11: fieldalignment: struct with 40 pointer bytes could be 24 (govet)
return struct {
^
nodebuilder/p2p/cmd/p2p.go:421:11: fieldalignment: struct with 40 pointer bytes could be 24 (govet)
return struct {
^
nodebuilder/share/cmd/share.go:60:11: fieldalignment: struct with 48 pointer bytes could be 24 (govet)
return struct {
^
header/headertest/fraud/testing.go:27:17: fieldalignment: struct with 56 pointer bytes could be 48 (govet)
type FraudMaker struct {
^
nodebuilder/tests/swamp/swamp.go:51:12: fieldalignment: struct with 536 pointer bytes could be 512 (govet)
type Swamp struct {
^
api/rpc_test.go:139:16: fieldalignment: struct with 16 pointer bytes could be 8 (govet)
var tests = []struct {
^
blob/blob.go:67:11: fieldalignment: struct with 88 pointer bytes could be 80 (govet)
type Blob struct {
^
blob/blob.go:111:15: fieldalignment: struct with 64 pointer bytes could be 56 (govet)
type jsonBlob struct {
^
blob/blob_test.go:22:15: fieldalignment: struct with 24 pointer bytes could be 16 (govet)
var test = []struct {
^
blob/service_test.go:50:15: fieldalignment: struct with 32 pointer bytes could be 24 (govet)
var test = []struct {
^
core/listener.go:32:15: fieldalignment: struct with 64 pointer bytes could be 56 (govet)
type Listener struct {
^
das/checkpoint.go:7:17: fieldalignment: struct with 32 pointer bytes could be 16 (govet)
type checkpoint struct {
^
das/checkpoint.go:17:23: fieldalignment: struct with 24 pointer bytes could be 8 (govet)
type workerCheckpoint struct {
^
das/coordinator.go:15:26: fieldalignment: struct with 224 pointer bytes could be 192 (govet)
type samplingCoordinator struct {
^
das/daser.go:24:12: fieldalignment: struct with 192 pointer bytes could be 152 (govet)
type DASer struct {
^
das/done.go:8:11: fieldalignment: struct with 24 pointer bytes could be 16 (govet)
type done struct {
^
das/state.go:12:23: fieldalignment: struct with 104 pointer bytes could be 40 (govet)
type coordinatorState struct {
^
das/state.go:43:19: fieldalignment: struct with 32 pointer bytes could be 24 (govet)
type retryAttempt struct {
^
das/stats.go:4:20: fieldalignment: struct with 40 pointer bytes could be 16 (govet)
type SamplingStats struct {
^
das/stats.go:24:18: fieldalignment: struct with 48 pointer bytes could be 24 (govet)
type WorkerStats struct {
^
das/worker.go:22:13: fieldalignment: struct with 128 pointer bytes could be 112 (govet)
type worker struct {
^
das/worker.go:43:10: fieldalignment: struct with 48 pointer bytes could be 16 (govet)
type job struct {
^
das/backoff_test.go:16:13: fieldalignment: struct with 48 pointer bytes could be 24 (govet)
tests := []struct {
^
das/backoff_test.go:50:13: fieldalignment: struct with 128 pointer bytes could be 112 (govet)
tests := []struct {
^
das/coordinator_test.go:333:18: fieldalignment: struct with 88 pointer bytes could be 56 (govet)
type mockSampler struct {
^
das/coordinator_test.go:446:17: fieldalignment: struct with 16 pointer bytes could be 8 (govet)
type checkOrder struct {
^
das/coordinator_test.go:520:11: fieldalignment: struct with 16 pointer bytes could be 8 (govet)
type lock struct {
^
das/daser_test.go:305:17: fieldalignment: struct with 40 pointer bytes could be 24 (govet)
type mockGetter struct {
^
header/headertest/testing.go:31:16: fieldalignment: struct with 56 pointer bytes could be 32 (govet)
type TestSuite struct {
^
libs/keystore/fs_keystore.go:18:17: fieldalignment: struct with 32 pointer bytes could be 24 (govet)
type fsKeystore struct {
^
libs/keystore/map_keystore.go:14:18: fieldalignment: struct with 32 pointer bytes could be 24 (govet)
type mapKeystore struct {
^
nodebuilder/config.go:27:13: fieldalignment: struct with 640 pointer bytes could be 608 (govet)
type Config struct {
^
nodebuilder/node.go:45:11: fieldalignment: struct with 296 pointer bytes could be 272 (govet)
type Node struct {
^
nodebuilder/store.go:143:14: fieldalignment: struct with 64 pointer bytes could be 48 (govet)
type fsStore struct {
^
nodebuilder/header/config.go:23:13: fieldalignment: struct with 232 pointer bytes could be 184 (govet)
type Config struct {
^
nodebuilder/p2p/bitswap.go:66:20: fieldalignment: struct with 64 pointer bytes could be 56 (govet)
type bitSwapParams struct {
funnily enough @Wondertan, the first PR i made @distractedm1nd said "oh, lint is failing on some alignment issue" and i thought it was this check and thought "damn, that is cool to run that as a CI check", and well...here we are.
Codecov Report
Attention: 1 lines in your changes are missing coverage. Please review.
Comparison is base (
150378f) 50.99% compared to head (6bb4018) 50.79%.
| Files | Patch % | Lines |
|---|---|---|
| share/eds/byzantine/share_proof.go | 50.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #2856 +/- ##
==========================================
- Coverage 50.99% 50.79% -0.20%
==========================================
Files 176 176
Lines 11172 11172
==========================================
- Hits 5697 5675 -22
- Misses 4974 4993 +19
- Partials 501 504 +3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
i had a nightmare about this open PR so came and figured i'd come and do all the alignment work so its simple and ready to go for you @Wondertan
noisy yes, but its exactly 69 files changed so "noice" too 😆
I found a good tool betteralignment that would make suggested alignment/padding fixes and preserve comments etc so we don't lose readability, i set it to ignore test files too as it would particularly flip out on named table driven tests.
Would you want to make a similar PR to go-header?
this one has lingered, is pretty monumental, disruptive, and the overall experience of both the linter pointing out these vet/field alignment issues and then trying to fix them kinda sucks. Rather than blanket requiring field alignment EVERYWHERE then marking as //nolint, we should align structs where performance is critical then we can add a test for those that are as and when