Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Protocol testing: test what happens when nodes join the network much later #231

Closed
edsko opened this issue Feb 7, 2019 · 2 comments · Fixed by #773
Closed

Protocol testing: test what happens when nodes join the network much later #231

edsko opened this issue Feb 7, 2019 · 2 comments · Fixed by #773
Assignees
@nfrisby
Labels
consensus issues related to ouroboros-consensus testing

Comments

@edsko
Copy link
Contributor

edsko commented Feb 7, 2019

Again, a very important test because this is the one that will exercise the genesis chain selection rule.
@edsko edsko added the consensus issues related to ouroboros-consensus label Feb 7, 2019
@edsko edsko added this to the Consensus_Testing milestone Feb 7, 2019
@edsko edsko mentioned this issue Dec 1, 2023
Open
@nfrisby nfrisby removed their assignment Jul 10, 2019
@mrBliss
Copy link
Contributor

mrBliss commented Jul 10, 2019

@nfrisby You can ignore the "genesis chain selection rule" for now.

A possible approach is to replace the NumCoreNodes argument that these tests take with a more complicated type (still as simple as possible and easy to shrink!), that defines at which time (just the slot, see onSlot) each node joins the network (this can be the time the node starts up or the time the network connection between the node and the others is made). The next step is to set up the nodes and connections based on this type. Eventually, we can extend this type to account for network partitions and other "problems".

This was referenced Jul 15, 2019
Merged
Open
Merged
@nfrisby
Copy link
Contributor

nfrisby commented Jul 23, 2019

Just an update for the record: the draft PR #773 found a race condition and a bug in newIterator. Thomas addressed those with #810 and #812. I'm following up his work with a proper PR for this Issue.

This was referenced Jul 24, 2019
Merged
Merged
@nfrisby nfrisby mentioned this issue Aug 5, 2019
Merged
iohk-bors bot added a commit that referenced this issue Aug 6, 2019
@iohk-bors @intricate @nfrisby
Merge #862 #880
59c73a9 
862: [#440] Implement "already validated" flag for block and tx application r=mrBliss a=intricate

Closes #440 

880: Use record type for output of protocol tests r=mrBliss a=nfrisby

`broadcastNetwork` currently just returns a map. In tickets #231 etc and related debugging/investigations, I've wanted programmatic access to more information from the run.

This PR is a general refinement that simplifies the types within each per-protocol family member's test code and makes it easier to add information to the general test framework.

Co-authored-by: Luke Nadur <[email protected]>
Co-authored-by: Nicolas Frisby <[email protected]>
This was referenced Aug 7, 2019
Merged
Merged
iohk-bors bot added a commit that referenced this issue Aug 13, 2019
@iohk-bors @nfrisby
Merge #916
b42077e 
916: fixup Real PBFT setup r=mrBliss a=nfrisby

The `RealPBFT` setup was creating nodes with different `CoreNodeId`s than expected. This was causing property failures during my work-in-progress on Issue #231 PR #773.

Is it safe to export `plcCoreNodeId`, even just for testing? I'm weary of adding exports to code I'm not familiar with.

Co-authored-by: Nicolas Frisby <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 13, 2019
@iohk-bors @nfrisby
Merge #916
54a4ff1 
916: fixup Real PBFT setup r=mrBliss a=nfrisby

The `RealPBFT` setup was creating nodes with different `CoreNodeId`s than expected. This was causing property failures during my work-in-progress on Issue #231 PR #773.

Is it safe to export `plcCoreNodeId`, even just for testing? I'm weary of adding exports to code I'm not familiar with.

Co-authored-by: Nicolas Frisby <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 13, 2019
@iohk-bors @mrBliss
@edsko @nfrisby @coot
Merge #914 #916 #918
826889b 
914: Replace the ThreadRegistry with the ResourceRegistry r=mrBliss a=mrBliss

Threads (and sub-`ThreadRegistry`s) are also resources, so we can use the `ResourceRegistry` to allocate/release them, instead of having two separate `-Registry`s.

TODO: consensus tests are failing with:
```
Exception:
        ResourceRegistry used after closing
        CallStack (from HasCallStack):
          error, called at src/Ouroboros/Consensus/Util/ResourceRegistry.hs:104:31 in ouroboros-consensus-0.1.0.0-6HDMr0z0KdXIwfzJlxCxiL:Ouroboros.Consensus.Util.ResourceRegistry
```

916: fixup Real PBFT setup r=mrBliss a=nfrisby

The `RealPBFT` setup was creating nodes with different `CoreNodeId`s than expected. This was causing property failures during my work-in-progress on Issue #231 PR #773.

Is it safe to export `plcCoreNodeId`, even just for testing? I'm weary of adding exports to code I'm not familiar with.

918: Eq and Ord instances for Async m a r=dcoutts a=coot

Using 'QuantifiedConstraints' extension in 'MonadAsync'.
Also provide instances for 'Async (SimM s) a'.

Co-authored-by: Thomas Winant <[email protected]>
Co-authored-by: Edsko de Vries <[email protected]>
Co-authored-by: Nicolas Frisby <[email protected]>
Co-authored-by: Marcin Szamotulski <[email protected]>
@nfrisby nfrisby mentioned this issue Aug 29, 2019
Closed
@nfrisby nfrisby mentioned this issue Dec 1, 2023
Open
iohk-bors bot added a commit that referenced this issue Sep 5, 2019
@iohk-bors @nfrisby
Merge #773
f79f071 
773: Have some nodes join later in test-consensus r=nfrisby a=nfrisby

This PR is only for discussion. I have a first draft take on #231 and would like feedback, of all kinds.

Edit: the PR has matured and looks nearly ready to merge, but I suspect I should open a separate PR because of the 100+ comments on this one.

Edit: Fixes #231.

Co-authored-by: Nicolas Frisby <[email protected]>
iohk-bors bot added a commit that referenced this issue Sep 5, 2019
@iohk-bors @nfrisby
Merge #773
776d09c 
773: Have some nodes join later in test-consensus r=intricate a=nfrisby

This PR is only for discussion. I have a first draft take on #231 and would like feedback, of all kinds.

Edit: the PR has matured and looks nearly ready to merge, but I suspect I should open a separate PR because of the 100+ comments on this one.

Edit: Fixes #231.

Co-authored-by: Nicolas Frisby <[email protected]>
iohk-bors bot added a commit that referenced this issue Sep 6, 2019
@iohk-bors @nfrisby
Merge #773
0cd865b 
773: Have some nodes join later in test-consensus r=intricate a=nfrisby

This PR is only for discussion. I have a first draft take on #231 and would like feedback, of all kinds.

Edit: the PR has matured and looks nearly ready to merge, but I suspect I should open a separate PR because of the 100+ comments on this one.

Edit: Fixes #231.

Co-authored-by: Nicolas Frisby <[email protected]>
iohk-bors bot added a commit that referenced this issue Sep 6, 2019
@iohk-bors @nfrisby
Merge #773
f214602 
773: Have some nodes join later in test-consensus r=nc6 a=nfrisby

This PR is only for discussion. I have a first draft take on #231 and would like feedback, of all kinds.

Edit: the PR has matured and looks nearly ready to merge, but I suspect I should open a separate PR because of the 100+ comments on this one.

Edit: Fixes #231.

Co-authored-by: Nicolas Frisby <[email protected]>
@iohk-bors iohk-bors bot closed this as completed in 8f5ef4d Sep 9, 2019
@mrBliss mrBliss removed this from the Consensus_ProtocolTesting milestone Nov 7, 2019
@mrBliss mrBliss removed the mainnet label Nov 7, 2019
@nfrisby nfrisby mentioned this issue Mar 31, 2020
Open
coot added a commit that referenced this issue May 16, 2022
@iohk-bors @mrBliss
@edsko @nfrisby @coot
Merge #914 #916 #918
d7c9b1c 
914: Replace the ThreadRegistry with the ResourceRegistry r=mrBliss a=mrBliss

Threads (and sub-`ThreadRegistry`s) are also resources, so we can use the `ResourceRegistry` to allocate/release them, instead of having two separate `-Registry`s.

TODO: consensus tests are failing with:
```
Exception:
        ResourceRegistry used after closing
        CallStack (from HasCallStack):
          error, called at src/Ouroboros/Consensus/Util/ResourceRegistry.hs:104:31 in ouroboros-consensus-0.1.0.0-6HDMr0z0KdXIwfzJlxCxiL:Ouroboros.Consensus.Util.ResourceRegistry
```

916: fixup Real PBFT setup r=mrBliss a=nfrisby

The `RealPBFT` setup was creating nodes with different `CoreNodeId`s than expected. This was causing property failures during my work-in-progress on Issue #231 PR #773.

Is it safe to export `plcCoreNodeId`, even just for testing? I'm weary of adding exports to code I'm not familiar with.

918: Eq and Ord instances for Async m a r=dcoutts a=coot

Using 'QuantifiedConstraints' extension in 'MonadAsync'.
Also provide instances for 'Async (SimM s) a'.

Co-authored-by: Thomas Winant <[email protected]>
Co-authored-by: Edsko de Vries <[email protected]>
Co-authored-by: Nicolas Frisby <[email protected]>
Co-authored-by: Marcin Szamotulski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nfrisby nfrisby

Labels
consensus issues related to ouroboros-consensus testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Have some nodes join later in test-consensus IntersectMBO/ouroboros-network
3 participants
@mrBliss @edsko @nfrisby