Skip to content

Fix accounts_proposed subscription receives twice message#672

Merged
cindyyan317 merged 1 commit intoXRPLF:developfrom
cindyyan317:sub22
Jun 8, 2023
Merged

Fix accounts_proposed subscription receives twice message#672
cindyyan317 merged 1 commit intoXRPLF:developfrom
cindyyan317:sub22

Conversation

@cindyyan317
Copy link
Contributor

@cindyyan317 cindyyan317 commented Jun 8, 2023

Client will receive two identical messages if their subscription's accounts_proposed has new transactions.
The old code traversed all the field in the transactions, Trying to convert it to account, then send the update accordingly.
For payment transaction like this:
{
"Account":"rh1HPuRVsYYvThxG2Bs1MfjmrVC73S16Fb",
"Amount":"40000000",
"Destination":"rDgGprMjMWkJRnJ8M5RXq3SXYD8zuQncPc",
"Fee":"20",
"Flags":2147483648,
"Sequence":13767283,
"SigningPubKey":"036F3CFFE1EA77C1EEC5DCCA38C83E62E3AC068F8A16369620AF1D609BA5A620B2",
"TransactionType":"Payment",
"TxnSignature":"30450221009BD0D563B24E50B26A42F30455AD21C3D5CD4D80174C41F7B54969FFC08DE94C02201FC35320B56D56D1E34D1D281D48AC68CBEDDD6EE9DFA639CCB08BB251453A87",
"hash":"F44393295DB860C6860769C16F5B23887762F09F87A8D1174E0FCFF9E7247F07"
}

"SignlingPubKey" can be converted to account too, in this case, the account's update will be sent twice.

The fix is to not to use public key convert.
We previously did not use the real transaction json, not really cover the code "RPCHelper.cpp", also add the real json to unittest.

@cindyyan317 cindyyan317 requested a review from godexsoft June 8, 2023 09:36
@cindyyan317 cindyyan317 marked this pull request as ready for review June 8, 2023 09:37
Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

Awesome! 💥🚀

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #672 (4e0bcd2) into develop (244337c) will increase coverage by 0.32%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #672      +/-   ##
===========================================
+ Coverage    29.92%   30.25%   +0.32%     
===========================================
  Files          140      140              
  Lines         7431     7436       +5     
  Branches      4420     4422       +2     
===========================================
+ Hits          2224     2250      +26     
+ Misses        3137     3109      -28     
- Partials      2070     2077       +7     

see 2 files with indirect coverage changes

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

Successfully merging this pull request may close these issues.

2 participants