Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update ipfs #1367

Merged
merged 1 commit into from
Sep 3, 2024
Merged

update ipfs #1367

merged 1 commit into from
Sep 3, 2024

Conversation

mvdan
Copy link
Contributor

@mvdan mvdan commented Aug 19, 2024

(see commit messages - please do not squash)

@mvdan mvdan requested review from p4u and altergui August 19, 2024 22:19
@p4u
Copy link
Member

p4u commented Aug 20, 2024

Can you fix the tests @mvdan ?

@mvdan
Copy link
Contributor Author

mvdan commented Aug 25, 2024

It looks like updating cometbft breaks something:

2024-08-19T22:31:45.5197948Z testsuite_10461785872-seed-1 | �[90m2024-08-19T22:22:53.074Z�[0m �[1m�[31mERR�[0m�[0m comet: MConnection panicked �[36merr=�[0m"log line with invalid chars: "\x1b[90m10:22PM\x1b[0m \x1b[1m\x1b[31mERR\x1b[0m\x1b[0m comet: tx already received from peer \x1b[36msender=\x1b[0mcb6a9612b39147cfc40e7514b334139004b7bf17 \x1b[36mtx=\x1b[0m\"W�:��2�{ڋUn-:�ы��ؖZWS��E]iE-\\x02\"\n"" �[36mstack=�[0m"goroutine 105 [running]:\nruntime/debug.Stack()\n\truntime/debug/stack.go:24 +0x67\ngithub.com/cometbft/cometbft/p2p/conn.(*MConnection)._recover(0xc007d74000)\n\tgithub.com/cometbft/[email protected]/p2p/conn/connection.go:341 +0x58\npanic({0x4f06760?, 0xc007a91400?})\n\truntime/panic.go:770 +0x132\ngo.vocdoni.io/dvote/log.(*invalidCharChecker).Write(0x0?, {0xc0065d1180, 0xc8, 0x380})\n\tgo.vocdoni.io/dvote/log/log.go:85 +0x145\nbytes.(*Buffer).WriteTo(0xc0099303c0, {0x607f220, 0x8b294e0})\n\tbytes/buffer.go:261 +0x122\ngithub.com/rs/zerolog.ConsoleWriter.Write({{0x607f220, 0x8b294e0}, 0x0, {0x0, 0x0}, {0xc009c88540, 0x4, 0x4}, {0x0, 0x0, ...}, ...}, ...)\n\tgithub.com/rs/[email protected]/console.go:145 +0x9be\ngithub.com/rs/zerolog.LevelWriterAdapter.WriteLevel(...)\n\tgithub.com/rs/[email protected]/writer.go:27\ngithub.com/rs/zerolog.multiLevelWriter.WriteLevel({{0xc000a8f500?, 0x69?, 0x280992dd15?}}, 0x3, {0xc0065ac200, 0xe6, 0x1f4})\n\tgithub.com/rs/[email protected]/writer.go:80 +0xea\ngithub.com/rs/zerolog.(*Event).write(0xc00094e070)\n\tgithub.com/rs/[email protected]/event.go:80 +0x2b3\ngithub.com/rs/zerolog.(*Event).msg(0xc00094e070, {0xc00a387560, 0x24})\n\tgithub.com/rs/[email protected]/event.go:151 +0x43a\ngithub.com/rs/zerolog.(*Event).Msg(...)\n\tgithub.com/rs/[email protected]/event.go:110\ngo.vocdoni.io/dvote/log.(*CometLogger).Error(0xc0008b3560, {0x5566c9d, 0x1d}, {0xc009c88480, 0x4, 0x4})\n\tgo.vocdoni.io/dvote/log/comet.go:47 +0x28e\ngithub.com/cometbft/cometbft/mempool.(*CListMempool).CheckTx(0xc0050300c0, {0xc0065c8dc0, 0x130, 0x130}, {0xc0006315c0, 0x28})\n\tgithub.com/cometbft/[email protected]/mempool/clist_mempool.go:277 +0x6a2\ngithub.com/cometbft/cometbft/mempool.(*Reactor).Receive(0xc000a4a580, {{0x60d7d08, 0xc007d6bd40}, {0x60953d0, 0xc00a313758}, 0x30})\n\tgithub.com/cometbft/[email protected]/mempool/reactor.go:155 +0xa59\ngithub.com/cometbft/cometbft/p2p.createMConnection.func1(0x30, {0xc007d76000, 0x136, 0x1000})\n\tgithub.com/cometbft/[email protected]/p2p/peer.go:429 +0x65b\ngithub.com/cometbft/cometbft/p2p/conn.(*MConnection).recvRoutine(0xc007d74000)\n\tgithub.com/cometbft/[email protected]/p2p/conn/connection.go:671 +0x992\ncreated by github.com/cometbft/cometbft/p2p/conn.(*MConnection).OnStart in goroutine 219\n\tgithub.com/cometbft/[email protected]/p2p/conn/connection.go:238 +0x3dd\n"

(very mangled panic, but you see the original error "comet: tx already received from peer")

I will undo the cometbft update and keep the ipfs update.

Along with all the IPFS, libp2p, and other updates that it brings.
For once, it seems like there are zero breaking changes for us.

Note that go-ipfs-blockstore has been moved to boxo/blockstore.
@mvdan mvdan changed the title update ipfs and cometbft update ipfs Aug 25, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 10546260405

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage remained the same at 62.243%

Files with Coverage Reduction New Missed Lines %
vochain/transaction/admin_tx.go 2 48.81%
vochain/transaction/transaction.go 2 59.23%
Totals Coverage Status
Change from base Build 10522335386: 0.0%
Covered Lines: 16520
Relevant Lines: 26541

💛 - Coveralls

@mvdan
Copy link
Contributor Author

mvdan commented Aug 25, 2024

Green now. We can try with cometbft separately.

@altergui
Copy link
Contributor

(very mangled panic, but you see the original error "comet: tx already received from peer")

in fact it looks like our "log encoding issues detector" found a log line with encoding issues from comet (because tx= is not hex, likely a fmt.Sprintf %s vs %x) and panics because CI is run with LOG_PANIC_ON_INVALIDCHARS

either we fix that encoding upstream 👀 or we disable our check

@p4u
Copy link
Member

p4u commented Sep 3, 2024

Created new issue for upgrading CometBFT #1378

@p4u p4u merged commit 88b17fc into vocdoni:main Sep 3, 2024
10 checks passed
@mvdan mvdan deleted the updates-2 branch September 11, 2024 19:28
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.

4 participants