-
Notifications
You must be signed in to change notification settings - Fork 338
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
Implement unit test for get_impacted_accounts() #183
Comments
Whoever writes test(s) to close this ticket will need to understand what the behavior of Suppose Alice places an order, which matches an existing order placed earlier by Bob. Then Alice's account is impacted by her transaction because the order operation's execution will change her account's balance and set of owned objects regardless of chain state. Even though the order changes Bob's account state as well -- his balances and his set of owned objects will be different -- Bob's account is not impacted by the order because Alice's order would also execute on a chain state in which Bob's order does not exist. This example clearly shows that in this context, the word "impacted" must have a specific technical meaning different from ordinary English usage. The ordinary English definition of the word "impacted" would lead one to believe that both Alice and Bob are impacted by the order; the fact that Bob is not impacted is a direct consequence of the API design, where "impacted" is determined by inspection of the transaction and cannot refer to chain state. Because As @nathanhourt points out, changing the amount an account is allowed to withdraw causes the account to be impacted, without directly changing its set of owned objects or balances. The definition of the word "impacted" must work with this wrinkle as well (or the implementation needs to be changed to conform to the definition.) We may be tempted to define "impacted accounts" to be the set of account ID's explicitly included in the operation. However, the
@nathanhourt also advises that an included-fields definition of "impacted" will open the door to tempting us to include account ID's in an operation solely because we want them to be returned by The real question, then, is where |
This design implies that there should be an API call to retroactively generate notifications for a client that has been offline, (kind of virtual notification queue). However, such an API has apparently not been implemented. |
Since it is only involved with API-level stuff (notifications and account history), I will move this logic out of the core and into the API. |
Possibly related: #166 |
This is no longer part of the core, as of the above commit it's part of the app / API layer since it's used for notifications, nothing in the core. |
I believe the intent is that clients can walk their accounts' operation history from most recent to last seen in order to get this queue. |
This issue was moved to bitshares/bitshares-core#44 |
The
get_impacted_accounts()
function needs to be tested with all operation types. This will be an enormous test!The text was updated successfully, but these errors were encountered: