TX Submission Logic #4887
Draft
TX Submission Logic #4887
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
coot
force-pushed
the
coot/tx-submission
branch
6 times, most recently
from
76c0d13 to
11d833b
Compare
This was referenced Sep 18, 2024
crocodile-dentist
force-pushed
the
coot/tx-submission
branch
2 times, most recently
from
772809e to
374a78a
Compare
coot
force-pushed
the
coot/tx-submission
branch
from
374a78a to
388cc69
Compare
Allow monadic action when trying to pipeline more messages, while collecting responses.
Export everything from the `Ouroboros.Network.TxSubmission.Inbound` module.
- Factors out common type definitions and their generators and properties to a Common.hs file. - Adds a TxSubmissionV2 file with the boilerplate to run the new, more accurate submission that uses the V2 version of TxSubmission protocol
Refactored SimResult name to not clash with IOSim's
- Updated txSubmissionV2 test - Fixed TODO about passing an STM action inside receivedTxIds - Fixed usage of partial function `(!)` - Fixed wrong usage of MVars in decisionLogicThread that lead to deadlock.
There was a race condition between the `decisionLogicThread` producing
the right policy and inbound server picking up the most up to date
policy. This would lead to the inbound side issuing a blocking request
when the client was awaiting for a non-blocking request. This blocking
request isn't wrong considering the policy it was used, it is a legit
decision that's made which leads to the inbound server issuing a
blocking request but because we have received a txid in the meantime and
didn't manage to read it soon enough we didn't create a more important
decision. The fix involved being aware of how many requests for txs we
have in flight and not generate "low priority" policies.
`hasTxIdsToAcknowledge` is not used anywhere in the code so it is
removed.
`filterActivePeers` is improved by making its decision logic more closed
to `pickTxsToDownload`.
`filterActivePeers` test is fixed, since it doesn't hold under the new
logic:
`filterActivePeers` will not compute a decision for peers which have
`requestedTxIdsInflight` and `makeDecisions` computes non-empty decisions
for peers with no `requestedTxIdsInflight`. So:
1. "The set of active peers is a superset of peers for which a decision
was made" this is not true since it is possible that a non active
peer has a legitimate decision, but due to our race-condition
protection condition we just don't generate it.
2. "The set of active peer which can acknowledge txids is a subset of
peers for which a decision was made" this is removed since
hasTxIdsToAcknowledge function is removed
3. "Decisions made from the results of `filterActivePeers` is the same
as from the original set" this isn't true because of what I said
above
So I refactored the test to check that the number of filtered decisions
is a subset of the total number of decisions, which I believe to be a
more accurate test for the current logic
Add `max_TX_SIZE` which is shared between * `defaultTxDecisionPolicy`, and * `txSubmissionProtocolLimits`
- Adds tx submission diffusion testnet test This test checks that even in the presence of a node that keeps disconnecting, but eventually stays online, we manage to learn about all its transactions.
Module structure needs to be reorganised to have just one debug tracer.
Use `IOSim` API. `evaluateTrace` from `Test.Ouroboros.Network.LedgerPeers` has the annoying property that once the trace was evaluated in won't show the trace again, which makes it hard to work with `cabal repl`. Refactored `checkMempools` to improve readablity. Should be squashed onto `c9d45673ca New txSubmissionV2 simulation`
This allows us to have just one tracer for tx-submission decision logic.
coot
force-pushed
the
coot/tx-submission
branch
from
efbc13a to
7b8ec92
Compare
* Put `TxLogic` tests in a separate module, and common API in `Types`.
* Fixed tasty test hierarchy.
* Renamed `TxSubmissionV{1,2}` as `AppV{1,2}`
Send TraceTxSubmissionProcessed for the new TX submission protocol. Fix count for TraceTxSubmissionCollected, it should be number of TXs "collected", not number of TXs accepted by the mempool.
coot
force-pushed
the
coot/tx-submission
branch
from
7b8ec92 to
22c71d5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a draft PR (work in progress) which introduces
tx-submissionlogic responsible for choosing from which peer to download a tx.Checklist
Quality
Maintenance
ouroboros-networkproject.