search

IntersectMBO / ouroboros-network

Specifications of network protocols and implementations of components running these protocols which support a family of Ouroboros Consesus protocols; the diffusion layer of the Cardano Node.
https://ouroboros-network.cardano.intersectmbo.org
Apache License 2.0
268 stars 86 forks source link

Connection Manager invalid transition #4891

Closed bolt12 closed 1 day ago

bolt12 commented 3 weeks ago 
  Ouroboros.Network.Testnet
    IOSimPOR
      connection manager valid transitions: FAIL (198.64s)
        *** Failed! Falsified (after 43 tests and 2787 shrinks):
        Schedule control: ControlDefault
        No thread delayed

        connection:
        UnknownConnectionSt            → ReservedOutboundSt
        ReservedOutboundSt             → UnnegotiatedSt Inbound
        UnnegotiatedSt Inbound         → UnnegotiatedSt Outbound
        UnnegotiatedSt Outbound        → InboundIdleSt Duplex
        InboundIdleSt Duplex           → OutboundDupSt Ticking
        OutboundDupSt Ticking          → DuplexSt

        Unexpected transition: UnnegotiatedSt Outbound → InboundIdleSt Duplex
        Use --quickcheck-replay=481904 to reproduce.

IOSimPOR found this test failure.

bolt12 commented 3 weeks ago

This pattern match ignores the provenance of the state so in the case of self connections, IOSimPOR found the following state transition:

        UnknownConnectionSt            → ReservedOutboundSt
        ReservedOutboundSt             → UnnegotiatedSt Inbound (Overwritten transition)
        UnnegotiatedSt Inbound         → UnnegotiatedSt Outbound (SelfConn-1)
        UnnegotiatedSt Outbound        → InboundIdleSt Duplex (After handshake on inbound side 1)
        InboundIdleSt Duplex           → OutboundDupSt Ticking (After handshake on outbound side 2 - This is also a SelfConn-1 transition according to the diagram)
        OutboundDupSt Ticking          → DuplexSt

In 1 and 2 there's a race where one runs first than the other. The network spec is a bit outdated since it does not include these self connection transitions. I am updating them accordingly. It seems to me this is not a problem, just a transition we failed to mark as possible in our tests.

Here's the test failure description: https://github.com/IntersectMBO/ouroboros-network/issues/4891

Also in our test suite there's this commented out: https://github.com/IntersectMBO/ouroboros-network/blob/master/ouroboros-network-framework/testlib/Ouroboros/Network/ConnectionManager/Test/Utils.hs#L108 In truth we should now enable this transitions and other since IOSimPOR is able to observe them :slightly_smiling_face: