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

Neaten and modernize PaymentAdjuster's integration #780

Open
bertllll opened this issue Mar 20, 2024 · 1 comment
Open

Neaten and modernize PaymentAdjuster's integration #780

bertllll opened this issue Mar 20, 2024 · 1 comment
Projects

Comments

@bertllll
Copy link

bertllll commented Mar 20, 2024

Awaiting features that are going to come available with the modernization following the Falling Behind! #676 card, it would be of great use in the PaymentAdjuster. In particular, the system of series of messages that precede the payment point is long and considerably complex. The old technologies required us to gradually save pieces of information inside different messages, while only part of the message has ever had a meaning for the message Handler and the rest is what we need to convey to the terminal destination, roughly put, assuming the accumulated amount of information is increasing with every other message. We've preferred this arrangement to using an Actor's state to cache partial data, with the Accountant in question.

The complexity can be sketched like:
(Includes cards not having been implemented yet, like: #779 and #699)

Accountant: RequestPaymentThresholdsMessage -> Neighborhood,
Neighborhood: CreditorsPaymentThresholdsMessage -> Accountant,
Accountant -> QualifiedPayablesMessage -> BlockchainBridge,
BlockchainBridge: BlockchainAgentWithContextMessage -> Accountant,
...if it is discovered that the PaymentAdjuster will be needed...
Accountant: NodeConnectivityAssesmentMessage (nonexistent by now) -> Neighborhood,
Neighborhood: AdjustmentSurveyCompletedMessage (nonexistent by now) -> Accountant,
...performing adjustment works...
Accountant: OutboundPaymentInstructions -> BlockchainBridge

...where each stage, therefore a message as well as handling point, draws a lot of extra code and probably also wider area of tests. This launched design could be additionally fought off with the technologies from the modernized Actix library.
As @dnwiebe once suggested, it has become even more oriented towards asynchronously handled messages (logically, if we regard the drift with the whole library in this direction), despite keeping also the old ways of doing around but their old brightness is almost gone. We can easily come to see the benefits of switch along with this drift, especially in this complex situation.

Rewritten in more up-to-date code, we could implement only a single Message Handler in the Accountant, compared to four in the old days. Thanks to asynchronous code we don't have to leave the handle function but can wait for a response to come back from the other Actors whose help the Accountant needs to complete the task.

Accountant: RequestPaymentThresholdsMessage -> Neighborhood -> and resuming when the Neighborhood answers.
Accountant -> QualifiedPayablesMessage -> BlockchainBridge -> and resuming when the BlockchainBridge answers.
...if it is discovered that the PaymentAdjuster will be needed...
Accountant: NodeConnectivityAssesmentMessage (nonexistent by now) -> Neighborhood -> and resuming on the event of answer.
Accountant: OutboundPaymentInstructions -> BlockchainBridge

This is going to be an opportunity to make the code briefer and enable any future works in this area to go much smoother.

@bertllll bertllll created this issue from a note in MASQNode (New) Mar 20, 2024
@dnwiebe dnwiebe moved this from New to Backlog in MASQNode Mar 24, 2024
@kauri-hero
Copy link

Blocked by #676

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Backlog
MASQNode
  
Backlog
Development

No branches or pull requests

2 participants