A peer has at most one route for a prefix, but both live route tables were unique on prefix, origin AS and peer. So a peer that changed a prefix's origin AS got a second row instead of a replacement, and a withdrawal sent in the same message as announcements was given their origin AS and missed the withdrawn route.
What changed:
Both live route tables are unique on prefix and peer.
An announcement updates that peer's row for the prefix, origin AS included.
A withdrawal deletes that peer's row for the prefix and no longer carries an origin AS. LiveRouteDeletion loses its origin field, DeleteLiveRoute its origin parameter, and the delete-by-origin statements are gone.
New test TestPrefixHandlerKeepsOneRoutePerPrefixAndPeer covers both cases from the issue for IPv4 and IPv6 prefixes, checking the in-memory route count and prefix distribution against the table. Its count and distribution checks moved into a helper shared with TestPrefixHandlerAppliesEachPeersLastUpdateInABatch.
New test TestLiveRouteTablesHoldOneRoutePerPrefixAndPeer checks that each table refuses a second route from a peer for a prefix, which the code never attempts. Both new tests fail on next.
Things the diff does not show:
A withdrawal also stops carrying the message's AS path, which nothing read.
processWithdrawal and processWithdrawalDirect were never called (marked "kept for potential future use"); I cut each down to the one delete by prefix and peer rather than deleting them.
A database file created before this change keeps the old constraint, because the schema only creates tables that are missing. There is no migration, per the issue.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/routewatch/issues/55.
A peer has at most one route for a prefix, but both live route tables were unique on prefix, origin AS and peer. So a peer that changed a prefix's origin AS got a second row instead of a replacement, and a withdrawal sent in the same message as announcements was given their origin AS and missed the withdrawn route.
What changed:
- Both live route tables are unique on prefix and peer.
- An announcement updates that peer's row for the prefix, origin AS included.
- A withdrawal deletes that peer's row for the prefix and no longer carries an origin AS. `LiveRouteDeletion` loses its origin field, `DeleteLiveRoute` its origin parameter, and the delete-by-origin statements are gone.
- New test `TestPrefixHandlerKeepsOneRoutePerPrefixAndPeer` covers both cases from the issue for IPv4 and IPv6 prefixes, checking the in-memory route count and prefix distribution against the table. Its count and distribution checks moved into a helper shared with `TestPrefixHandlerAppliesEachPeersLastUpdateInABatch`.
- New test `TestLiveRouteTablesHoldOneRoutePerPrefixAndPeer` checks that each table refuses a second route from a peer for a prefix, which the code never attempts. Both new tests fail on `next`.
Things the diff does not show:
- A withdrawal also stops carrying the message's AS path, which nothing read.
- `processWithdrawal` and `processWithdrawalDirect` were never called (marked "kept for potential future use"); I cut each down to the one delete by prefix and peer rather than deleting them.
- A database file created before this change keeps the old constraint, because the schema only creates tables that are missing. There is no migration, per the issue.
Model: opus-5-5
Finding: the IPv6 half of the fix is not tested. In internal/database/database.go, updateLiveRouteV6SQL and upsertRouteRowV6 changed the same way as the IPv4 ones. TestPrefixHandlerKeepsOneRoutePerPrefixAndPeer uses only IPv4 prefixes, and no other test has a peer re-announce an IPv6 prefix with a new origin AS. So the IPv6 update could go on matching on origin AS without any test failing. If it did, an origin change would break the new UNIQUE(prefix, peer_ip) constraint and the whole batch would fail. Acceptable: a test that fails if the IPv6 path keeps the old behaviour. One option is a step in TestPrefixDistributionTracksWrites (internal/database/counts_test.go, which already runs IPv4 and IPv6 through both batch and single-route writes) where a peer re-announces an IPv4 and an IPv6 prefix with a new origin AS. It should check the origin AS stored, the route counts and the distribution. Another is IPv6 cases in the new handler test.
Model: opus-5-5
**Finding: the IPv6 half of the fix is not tested.** In `internal/database/database.go`, `updateLiveRouteV6SQL` and `upsertRouteRowV6` changed the same way as the IPv4 ones. `TestPrefixHandlerKeepsOneRoutePerPrefixAndPeer` uses only IPv4 prefixes, and no other test has a peer re-announce an IPv6 prefix with a new origin AS. So the IPv6 update could go on matching on origin AS without any test failing. If it did, an origin change would break the new `UNIQUE(prefix, peer_ip)` constraint and the whole batch would fail. Acceptable: a test that fails if the IPv6 path keeps the old behaviour. One option is a step in `TestPrefixDistributionTracksWrites` (`internal/database/counts_test.go`, which already runs IPv4 and IPv6 through both batch and single-route writes) where a peer re-announces an IPv4 and an IPv6 prefix with a new origin AS. It should check the origin AS stored, the route counts and the distribution. Another is IPv6 cases in the new handler test.
Model: opus-5-5
A peer has at most one route for a prefix, but the live route tables were
keyed by prefix, origin AS and peer. An announcement with a new origin AS
added a second row instead of replacing the first, and a withdrawal sent in
the same message as announcements was given their origin AS and so missed
the withdrawn route.
The tables are now unique on prefix and peer. An announcement updates the
peer's row for the prefix, origin AS included, and a withdrawal deletes it
without looking at an origin AS. The origin-only delete statements, the
deletion's origin field and DeleteLiveRoute's origin parameter are removed.
Model: opus-5-5
IPv6 finding: TestPrefixHandlerKeepsOneRoutePerPrefixAndPeer now runs both issue cases with IPv6 prefixes too, checking origin AS, route counts and distribution; added TestLiveRouteTablesHoldOneRoutePerPrefixAndPeer for the constraint on both tables; rebased onto next.
Model: opus-5-5
IPv6 finding: `TestPrefixHandlerKeepsOneRoutePerPrefixAndPeer` now runs both issue cases with IPv6 prefixes too, checking origin AS, route counts and distribution; added `TestLiveRouteTablesHoldOneRoutePerPrefixAndPeer` for the constraint on both tables; rebased onto `next`.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #55.
A peer has at most one route for a prefix, but both live route tables were unique on prefix, origin AS and peer. So a peer that changed a prefix's origin AS got a second row instead of a replacement, and a withdrawal sent in the same message as announcements was given their origin AS and missed the withdrawn route.
What changed:
LiveRouteDeletionloses its origin field,DeleteLiveRouteits origin parameter, and the delete-by-origin statements are gone.TestPrefixHandlerKeepsOneRoutePerPrefixAndPeercovers both cases from the issue for IPv4 and IPv6 prefixes, checking the in-memory route count and prefix distribution against the table. Its count and distribution checks moved into a helper shared withTestPrefixHandlerAppliesEachPeersLastUpdateInABatch.TestLiveRouteTablesHoldOneRoutePerPrefixAndPeerchecks that each table refuses a second route from a peer for a prefix, which the code never attempts. Both new tests fail onnext.Things the diff does not show:
processWithdrawalandprocessWithdrawalDirectwere never called (marked "kept for potential future use"); I cut each down to the one delete by prefix and peer rather than deleting them.Model: opus-5-5
35d225f6e9to4522b46616Finding: the IPv6 half of the fix is not tested. In
internal/database/database.go,updateLiveRouteV6SQLandupsertRouteRowV6changed the same way as the IPv4 ones.TestPrefixHandlerKeepsOneRoutePerPrefixAndPeeruses only IPv4 prefixes, and no other test has a peer re-announce an IPv6 prefix with a new origin AS. So the IPv6 update could go on matching on origin AS without any test failing. If it did, an origin change would break the newUNIQUE(prefix, peer_ip)constraint and the whole batch would fail. Acceptable: a test that fails if the IPv6 path keeps the old behaviour. One option is a step inTestPrefixDistributionTracksWrites(internal/database/counts_test.go, which already runs IPv4 and IPv6 through both batch and single-route writes) where a peer re-announces an IPv4 and an IPv6 prefix with a new origin AS. It should check the origin AS stored, the route counts and the distribution. Another is IPv6 cases in the new handler test.Model: opus-5-5
4522b46616to431d1d0481431d1d0481to7b96f2a7eb7b96f2a7ebto85471cf100IPv6 finding:
TestPrefixHandlerKeepsOneRoutePerPrefixAndPeernow runs both issue cases with IPv6 prefixes too, checking origin AS, route counts and distribution; addedTestLiveRouteTablesHoldOneRoutePerPrefixAndPeerfor the constraint on both tables; rebased ontonext.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.