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

move signal ignore out of eth/common #734

Merged
merged 3 commits into from
Sep 24, 2024
Merged

move signal ignore out of eth/common #734

merged 3 commits into from
Sep 24, 2024

Conversation

arnetheduck
Copy link
Member

net/nat is the only place where it is used and it certainly doesn't belong in common

`net/nat` is the only place where it is used and it certainly doesn't
belong in `common`
eth/net/nat.nim Outdated Show resolved Hide resolved
@tersec
Copy link
Contributor

tersec commented Sep 24, 2024

 [NimScript] exec: nim c  --styleCheck:usages --styleCheck:error --verbosity:0 --hints:off --skipUserCfg --nimcache:build/nimcache -f --warning[ObservableStores]:off -d:nimOldCaseObjects -d:chronicles_log_level=TRACE --threads:on -d:release  --mm:refc -r --outdir:build/p2p tests/p2p/all_tests
/Users/runner/work/nim-eth/nim-eth/eth/p2p/rlpx.nim(972, 1) template/generic instantiation of `p2pProtocol` from here
/Users/runner/work/nim-eth/nim-eth/eth/p2p/rlpx.nim(978, 14) template/generic instantiation of `append` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(311, 15) template/generic instantiation of `appendImpl` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(159, 11) template/generic instantiation of `append` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(311, 15) template/generic instantiation of `appendImpl` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(296, 7) template/generic instantiation of `appendRecordType` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(293, 21) template/generic instantiation of `enumerateRlpFields` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/object_serialization.nim(17, 9) template/generic instantiation of `op` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(291, 13) template/generic instantiation of `append` from here
/Users/runner/work/nim-eth/nim-eth/eth/rlp/writer.nim(308, 16) Warning: Signed integers cannot reliably be encoded using RLP [User]
/Users/runner/work/nim-eth/nim-eth/eth/p2p/peer_pool.nim(112, 57) template/generic instantiation of `async` from here
/Users/runner/work/nim-eth/nim-eth/eth/p2p/peer_pool.nim(152, 12) template/generic instantiation of `setResult` from here
/Users/runner/work/nim-eth/nim-eth/eth/p2p/peer_pool.nim(133, 25) Error: type mismatch
Expression: inc(rlpx_connect_success)
  [1] rlpx_connect_success: typedesc[IgnoredCollector]

Expected one of (first mismatch at [position]):
[1] func inc(a: var StInt; w: Word = 1)
[1] func inc(a: var StUint; w: Word = 1)
[1] proc inc(address: var TransportAddress; v: int = 1)
[1] proc inc[A](t: CountTableRef[A]; key: A; val = 1)
[1] proc inc[A](t: var CountTable[A]; key: A; val = 1)
[1] proc inc[T, V: Ordinal](x: var T; y: V = 1)

which is presumably because the import/export network dropped metrics here but it was never the right place to pull it in

if peer_pool wants metrics, it should import metrics itself

@arnetheduck arnetheduck merged commit 3d51887 into master Sep 24, 2024
18 checks passed
@arnetheduck arnetheduck deleted the move-ignore-sig branch September 24, 2024 09:30
@arnetheduck
Copy link
Member Author

arnetheduck commented Sep 24, 2024

if peer_pool wants metrics, it should import metrics itself

turns out it's using public gauges declared in: eth/p2p/rlpx.nim

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.

3 participants