Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#28442 closed defect (fixed)

1. sbws does not round bandwidths to 3 significant figures

Reported by: teor Owned by: teor
Priority: Medium Milestone: sbws: 1.0.x-final
Component: Core Tor/sbws Version:
Severity: Normal Keywords: sbws-1.0-must-closed-moved-20181128
Cc: pastly, juga Actual Points:
Parent ID: #27689 Points:
Reviewer: Sponsor:

Description (last modified by teor)

sbws currently rounds bandwidths to the nearest 1000. (Bandwidths under 500 are rounded to 1.)

This creates large errors in relay bandwidths:

  • max(round(499, -3), 1) = 1
  • max(round(500, -3), 1) = 1000
  • max(round(1499, -3), 1) = 1000

But torflow rounds bandwidths to 3 significant figures:
https://gitweb.torproject.org/torflow.git/tree/NetworkScanners/BwAuthority/aggregate.py#n62

And proposal 276 says to round to 2 significant figures, with some extra rounding of the final non-zero digit:
https://gitweb.torproject.org/torspec.git/tree/proposals/276-lower-bw-granularity.txt#n36

This is a bug on #27337.

Child Tickets

Change History (15)

comment:1 Changed 11 months ago by teor

Description: modified (diff)
Status: assignedneeds_review
Summary: sbws does not round bandwidths to 2 significant figuressbws does not round bandwidths to 3 significant figures

comment:2 Changed 11 months ago by juga

Status: needs_reviewneeds_revision

lgtm, just left a comment

comment:3 Changed 11 months ago by teor

Status: needs_revisionneeds_review

I improved the tests, let's make sure the CI passes, then we're good.

comment:4 Changed 11 months ago by teor

I squashed the branch and pushed it as bug28442-squashed on https://github.com/teor2345/sbws.git

comment:5 Changed 11 months ago by teor

Parent ID: #27689
Summary: sbws does not round bandwidths to 3 significant figures1. sbws does not round bandwidths to 3 significant figures

comment:6 Changed 11 months ago by teor

I also explained the bug better in the changelog.

comment:7 Changed 11 months ago by juga

Status: needs_reviewmerge_ready

comment:8 Changed 11 months ago by juga

Resolution: fixed
Status: merge_readyclosed

merged

comment:9 Changed 11 months ago by starlight

https://github.com/torproject/sbws/commit/44a588eeb22bfe6291c3e897723e0524993da396

to me it seems the patch rounds to three significant digits or the nearest 1000000 bytes/second where Torflow rounds to three significant digits or the nearest 1000 bytes/second

published values appear to confirm that

from 2018/11/18 20:00 UTC

Faravahar
---------
10000 deinemudda
10000 mad19
10000 tjwtechsvr1
9980 Antigona
9950 BraveKhafre
9950 ObiGeek
9940 angeltest2
9930 FreeCityofHH
9920 Vibescu
9920 idfTor1N1


longclaw
========
10000 yesno
10000 yoshihisa
10000 yuicat2
10000 zech1989
10000 zil3
9000 0x42FF
9000 1chandotus
9000 34FG46QWVK
9000 4XURGG5YCX
9000 Abbey1
9000 April
9000 Arrakis

comment:10 in reply to:  9 Changed 11 months ago by juga

Replying to starlight:

published values appear to confirm that

published values are using the Debian package, which is version 1.0.2, and that commit belongs to 1.0.3-dev0.
Thanks for the report.

comment:11 Changed 11 months ago by juga

I'll make a new version of stem asap, and Debian package will be updated soon, hopefully within a week.

comment:12 Changed 11 months ago by atagar

Hi juga. What do you mean by "I'll make a new version of stem asap"? I just want to confirm that we're not replacing the 1.7.0 deb Stem users install from with the untagged git repository version. ;)

comment:13 in reply to:  12 Changed 11 months ago by juga

Replying to atagar:

Hi juga. What do you mean by "I'll make a new version of stem asap"? I just want to confirm that we're not replacing the 1.7.0 deb Stem users install from with the untagged git repository version. ;)

ops, s/stem/sbws/
Sorry for the confusion.

comment:14 Changed 11 months ago by atagar

Ahhh! Whew, no worries. Thanks juga. :P

comment:15 Changed 11 months ago by teor

Keywords: sbws-1.0-must-closed-moved-20181128 added
Milestone: sbws 1.0 (MVP must)sbws: 1.0.x-final

Move all closed sbws 1.0 must tickets to sbws 1.0.x-final

Note: See TracTickets for help on using tickets.