Skip to content

Conversation

amsanghi
Copy link
Contributor

@amsanghi amsanghi commented Oct 16, 2025

Fixes: NIT-4017

The idea is we should log error, and now continue processing the message when signature errors out, so we just skip the message if signature errors out.

@amsanghi amsanghi marked this pull request as ready for review October 16, 2025 09:53
@amsanghi amsanghi requested a review from eljobe October 16, 2025 09:54
Copy link

github-actions bot commented Oct 16, 2025

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
2144 5 2139 0
View the top 3 failed tests by shortest run time
TestOutOfGasInStorageCacheFlush
Stack Traces | 3.040s run time
... [CONTENT TRUNCATED: Keeping last 20 lines]
=== CONT  TestOutOfGasInStorageCacheFlush
    program_test.go:2813: goroutine 687290 [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({0x40837d0, 0xc0a0bc0a80}, {0x4041580, 0xc0a3dde6c0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc0a0bc0a80, {0x4041580, 0xc0a3dde6c0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1723 +0x5d
        github.com/offchainlabs/nitro/system_tests.TestOutOfGasInStorageCacheFlush(0xc0a0bc0a80)
        	/home/runner/work/nitro/nitro/system_tests/program_test.go:2813 +0xfe6
        testing.tRunner(0xc0a0bc0a80, 0x3ccb6a8)
        	/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
        
    program_test.go:2813: �[31;1m [] failed calculating position for validation: batch not found on L1 yet �[0;0m
ERROR[10-17|09:51:16.757] Dangling trie nodes after full cleanup
INFO [10-17|09:51:16.764] HTTP server stopped                      endpoint=127.0.0.1:36561
TRACE[10-17|09:51:16.764] P2P networking is spinning down
--- FAIL: TestOutOfGasInStorageCacheFlush (3.04s)
TestPrimaryToSecondaryFailover
Stack Traces | 5.430s run time
=== RUN   TestPrimaryToSecondaryFailover
=== PAUSE TestPrimaryToSecondaryFailover
=== CONT  TestPrimaryToSecondaryFailover
INFO [10-17|09:42:57.356] arbitrum websocket broadcast server is listening address=[::]:42863
    broadcastclients_test.go:203: Primary broadcaster listening on: [::]:42863
INFO [10-17|09:42:57.356] arbitrum websocket broadcast server is listening address=[::]:44101
    broadcastclients_test.go:204: Secondary broadcaster listening on: [::]:44101
    broadcastclients_test.go:227: Primary URL: ws://127.0.0.1:42863
    broadcastclients_test.go:228: Secondary URL: ws://127.0.0.1:44101
INFO [10-17|09:42:57.361] connecting to arbitrum inbox message broadcaster url=ws://127.0.0.1:42863
INFO [10-17|09:42:57.364] Feed connected                           feedServerVersion=2 chainId=1234 requestedSeqNum=0
    broadcastclients_test.go:275: Phase 1: Sending messages from primary broadcaster
INFO [10-17|09:43:02.573] secondary feed started                   url=ws://127.0.0.1:44101 startingFromSeq=4
INFO [10-17|09:43:02.573] connecting to arbitrum inbox message broadcaster url=ws://127.0.0.1:44101
INFO [10-17|09:43:02.574] Feed connected                           feedServerVersion=2 chainId=1234 requestedSeqNum=4
    broadcastclients_test.go:307: Timed out waiting for message 5/5 from primary
--- FAIL: TestPrimaryToSecondaryFailover (5.43s)
TestVersion40
Stack Traces | 7.170s run time
... [CONTENT TRUNCATED: Keeping last 20 lines]
�[38;5;48;1myay!! we validated block 3 in 221.38ms�[0;0m
    precompile_inclusion_test.go:90: goroutine 454801 [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({0x40837d0, 0xc05df76e00}, {0x4041a00, 0xc09a4a3ce0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc05df76e00, {0x4041a00, 0xc09a4a3ce0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1723 +0x5d
        github.com/offchainlabs/nitro/system_tests.testPrecompiles(0xc05df76e00, 0x28, {0xc06f621df8, 0x5, 0x39?})
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:90 +0x371
        github.com/offchainlabs/nitro/system_tests.TestVersion40(0xc05df76e00?)
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:71 +0x64b
        testing.tRunner(0xc05df76e00, 0x3ccbb80)
        	/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
�[90mink price=9841�[0;0m
--- FAIL: TestVersion40 (7.17s)

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

@eljobe eljobe assigned tsahee and unassigned eljobe Oct 16, 2025
@eljobe eljobe requested a review from tsahee October 16, 2025 13:13
tsahee
tsahee previously approved these changes Oct 16, 2025
Copy link
Contributor

@tsahee tsahee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tsahee tsahee assigned eljobe and unassigned tsahee Oct 16, 2025
@amsanghi amsanghi requested a review from eljobe October 16, 2025 15:11
@eljobe
Copy link
Member

eljobe commented Oct 17, 2025

@amsanghi there appear to be merge conflicts. Can you take a look?

@eljobe eljobe assigned amsanghi and unassigned eljobe Oct 17, 2025
@amsanghi
Copy link
Contributor Author

@amsanghi there appear to be merge conflicts. Can you take a look?

Done.

@eljobe eljobe assigned eljobe and unassigned amsanghi Oct 17, 2025
@amsanghi amsanghi assigned eljobe and unassigned eljobe Oct 17, 2025
@eljobe eljobe added this pull request to the merge queue Oct 17, 2025
Merged via the queue into master with commit 9137187 Oct 17, 2025
21 of 22 checks passed
@eljobe eljobe deleted the signature_not_fatal branch October 17, 2025 10:12
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