Add API call to get general network statistics#1100
Add API call to get general network statistics#1100n1bor wants to merge 1 commit intoACINQ:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1100 +/- ##
==========================================
+ Coverage 82.56% 82.57% +0.01%
==========================================
Files 101 101
Lines 7697 7708 +11
Branches 302 306 +4
==========================================
+ Hits 6355 6365 +10
- Misses 1342 1343 +1
|
| (updateCount: Long,avgCltvExpiryDelta: Long,avgHtlcMinimumMsat: Long,avgFeeBaseMsat: Long,avgFeeProportionalMillionths: Long,avgHtlcMaximumMsat: Long, mf: FlagCounter, cf:FlagCounter ) <- allUpdates(None).map{ | ||
| updates => updates.foldLeft(Tuple8(0L, 0L, 0L, 0L, 0L, 0L,FlagCounter(), FlagCounter())) { | ||
| (t, c) => | ||
| Tuple8(t._1 + 1, |
There was a problem hiding this comment.
Instead of this hard-to-read Tuple8 why don't you directly use a GetNetworkInfoResponse here?
I think it would be cleaner.
There was a problem hiding this comment.
Not sure this is possible. As GetNetworkInfoResponse include results of the earlier futures too.
Could fill with dummy zeros but think that would make more confusing.
Especially if additional calls/futures were added to get more data.
There was a problem hiding this comment.
I would do something like (I slightly renamed the fields, you'll have to adapt):
override def getNetworkInfoResponse()(implicit timeout: Timeout): Future[GetNetworkInfoResponse] = {
for {
nodeCount <- allNodes().map(_.size)
channelCount <- allChannels().map(_.size)
channelUpdates <- allUpdates(None)
val networkInfo = channelUpdates.foldLeft(GetNetworkInfoResponse(channelCount, nodeCount, channelUpdates.size, 0, MilliSatoshi(0), MilliSatoshi(0), 0, MilliSatoshi(0))) {
(current, next) => current.copy(
avgCltvExpiryDelta = current.avgCltvExpiryDelta + next.cltvExpiryDelta,
avgFeeBaseMsat = current.avgFeeBaseMsat + next.feeBaseMsat
// etc
)
}
} yield networkInfo.copy(
avgFeeBaseMsat = networkInfo.avgFeeBaseMsat / networkInfo.updates
// etc
)
}
And I would refactor to make dividing all avg fields by the updateCount be done in a method in the case class or something like that.
eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala
Outdated
Show resolved
Hide resolved
t-bast
left a comment
There was a problem hiding this comment.
Thanks for your contribution, I think this is a useful addition to the API and we can enrich this response later (I'm thinking that average channel capacity would be useful to get).
I have a couple of code clean-up comments but concept ack.
eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala
Outdated
Show resolved
Hide resolved
| f.router.expectMsg('channels) | ||
| val channelId_ab = ShortChannelId(420000, 1, 0) | ||
| val channelId_bc = ShortChannelId(420000, 2, 0) | ||
| val chan_ab = fr.acinq.eclair.router.BaseRouterSpec.channelAnnouncement(channelId_ab, randomKey, randomKey, randomKey, randomKey) |
There was a problem hiding this comment.
nit: remove fr.acinq.eclair.router
There was a problem hiding this comment.
Am in EclairImpSpec so do need it. Wanted to make clear was coming from another Spec.
allchannels is now so slow it timesout.
This API call returns general stats on the network that is useful to know.