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

[ANCHOR-793] Update customer callback logic and implementation #1475

Open
Ifropc opened this issue Aug 27, 2024 · 5 comments
Open

[ANCHOR-793] Update customer callback logic and implementation #1475

Ifropc opened this issue Aug 27, 2024 · 5 comments

Comments

@Ifropc
Copy link
Contributor

Ifropc commented Aug 27, 2024

Background information

Currently, there are a few flaws with customer callback logic:
First,
Business server must always call notify_customer_info_updated on status update, even if the original request comes from the Anchor Platform, as part of PUT callback. If business doesn't call RPC, transaction status is not updated appropriately.
We make a callback to wallet both in RPC and in PUT /customer that leads to duplicate update on the wallet size.
Second, according to the SEP-12 spec, we should only send callbacks when status has been changed. Currently, we send callback on any PUT request and any RPC call (that is also done as part of PUT request handling).

Business server PUT callback proposal change

We propose to change PUT callback for the business server. It should now return full Customer body response. This will allow us to not require business to call RPC on sync updates anymore. Instead, we can perform the same logic in PUT handler (update transaction status)
Second, we should add a new required boolean field to Business Server PUT callback response (status_updated). When set to true, we will fire an customer status change event. If set to false, nothing happens.
In return, as RPC is used to sync updates, we should always fire customer status change events (the way it's done now)

@Ifropc Ifropc changed the title Update customer callback logic and implementation [ANCHOR-793] Update customer callback logic and implementation Aug 27, 2024
@lijamie98
Copy link
Collaborator

Second, we should add a new required boolean field to Business Server PUT callback response (status_updated). When set to true, we will fire an customer status change event. If set to false, nothing happens.

Why do we need the flag? If the AP sets the flag to true, AP already know if it should fire the status change event. If false, the AP also know it will not fire the event.

@lijamie98
Copy link
Collaborator

lijamie98 commented Aug 27, 2024

We propose to change PUT callback for the business server. It should now return full Customer body response. This will allow us to not require business to call RPC on sync updates anymore. Instead, we can perform the same logic in PUT handler (update transaction status)

In the sync case, what do we do if the BS calls the RPC? Can we elaborate?

@Ifropc
Copy link
Contributor Author

Ifropc commented Aug 27, 2024

In the sync case, what do we do if the BS calls the RPC? Can we elaborate?

We don't need to do anything, it's not a problem if BS calls the RPC. We just note in docs, that BS should not call RPC in the sync case (if they do, it will just create a duplicate event, that's it)

@Ifropc
Copy link
Contributor Author

Ifropc commented Aug 27, 2024

Why do we need the flag? If the AP sets the flag to true, AP already know if it should fire the status change event. If false, the AP also know it will not fire the event.

It's in the response of business server, we don't know the value of it ahead of time

@lijamie98
Copy link
Collaborator

Instead, we can perform the same logic in PUT handler (update transaction status)
I don't think this is a good idea because updating the transaction should be handled via RPC.

I propose that we only publish the customer callback in the PUT /customer endpoint of Sep12Service when the status_updated is true. The notify_customer_info_updated RPC handler should not trigger a callback event and only handle transaction status update.

This should prevent duplicated events.

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

No branches or pull requests

2 participants