tor-circmgr: If necessary, extend the circuit to become STUB+.

This addresses a TODO from hspool: if the pool returns a STUB circuit but we actually need a STUB+ one, we extend the circuit we were given by 1 hop

Closes #1400 (closed) and #1409 (closed)

requested review from @nickm

assigned to @gabi-250

This looks reasonable, but I think we should investigate the shadow-integration failure before merging.

Ah, the debug_assert_eq!(circ_path.hops().len(), 3) assertion is failing (2 != 3).

The shadow tests use lite vanguards, and for lite vanguards, the length of the stub circuit is always 2 because of a bug in VanguardHsPathBuilder::pick_path. I'll fix it in this branch...

In 69a5bc71 I added some extra assertions to catch this bug sooner (right after the path is created). 0d8efbd8 contains the actual fix (there are 2 other commits between them with some small intermediate refactorings)

I'll open a ticket for adding shadow tests for full vanguards (we currently only test with lite vanguards)

added 4 commits

• 69a5bc71 - tor-circmgr: After building the HS path, assert the length is correct.
• dcf20954 - tor-circmgr: Rename neighbor_exclusion variables for clarity.
• 72198255 - tor-circmgr: Assign error-handling closure to variable.
• 0d8efbd8 - tor-circmgr: Fix path not being extended if lite vanguards are enabled.

Compare with previous version

added 1 commit

• 58183ed1 - tor-circmgr: Reword a somewhat inaccurate comment about vanguards.

Compare with previous version

requested review from @nickm

Looks good now!

If the stub circuits were always length 2, should we call that a bug in a released version? If so let's open a ticket for it.

If the stub circuits were always length 2, should we call that a bug in a released version? If so let's open a ticket for it.

Yep, they were always of length 2. I'll open a bug ticket and link it to the MR.

This is #1409 (closed)

mentioned in issue #1409 (closed)

changed the description

approved this merge request

Sounds good; please merge when appropriate.

Thanks! This is ready now, so I'll go ahead and merge.

merged

mentioned in commit da95138c