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.

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

simplify the LSQ server test #937

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nfrisby
Copy link
Contributor

@nfrisby nfrisby commented Feb 2, 2024

For example, I removed the replication of the Immutable/VolatileTip acquire messages, since the test is so simple that the number of those is currently irrelevant---it's misleading to replicate them, as if that was already expected to matter.

For example, I removed the replication of the Immutable/VolatileTip acquire
messages, since the test is so simple that the number of those is currently
irrelevant---it's misleading to replicate them, as if that was already expected
to matter.
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

(Before merging, the "WIP" from the commit message should ofc be removed.)

@@ -144,7 +148,7 @@ checkOutcome k chain = conjoin . map (uncurry checkResult)
| otherwise
-> tabulate "Acquired" ["AcquireFailurePointNotOnChain"] $ property True
Left AcquireFailurePointTooOld
| pointSlot pt >= immutableSlot
| pointSlot pt >= immutableSlot -- TODO what if the immtip is a multi-leader slot?
Copy link
Member

@amesgen amesgen Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we ask for a point with the same slot as the immutable tip, but with a different hash, we get a AcquireFailurePointNotOnChain, right?

Nothing
| pointSlot pt < pointSlot immutablePoint
-> SendMsgFailure AcquireFailurePointTooOld idle
| otherwise
-> SendMsgFailure AcquireFailurePointNotOnChain idle

Comment on lines +91 to +92
-- A random sequence of targets: one for each block in the tree and also a
-- random number of immtip/voltip queries
Copy link
Member

@amesgen amesgen Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A random sequence of targets: one for each block in the tree and also a random number of immtip/voltip queries

Isn't the change that is explained in the PR description exactly about removing the fact that there is a random number of voltip queries, instead just using one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants