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

fix(core-api): include multipayment in forged amount #3733

Merged
merged 13 commits into from
Jun 1, 2020

Conversation

rainydio
Copy link
Contributor

@rainydio rainydio commented May 25, 2020

Summary

Fixes #3495

Block totalAmount property is signed and is used to checksum database. It cannot be changed to include multipayment transactions without milestone. So this is implemented on API level, but things there aren't that simple either.

I cannot simply pull all transactions with every block that is loaded from database. Otherwise this will significantly stress database on /blocks and /blocks/search endpoints. Second there is raw=true parameter which presumably still has to return raw block data that can be used to check signature (so without multipayment included in total).

  1. BlockHistoryService now has capability of joining transactions (...byCriteriaJoinTransactions methods).
  2. New BlockWithTransactionsResource was added. Only its transform method is used which walks through transactions and calculates forged total that includes multipayment.
  3. All of transformed BlockResource usages are now using BlockWithTransactionsResource.
  4. Those usages requiring transformed BlockWithTransactionsResource use new ..byCriteriaJoinTransactions methods to also join multipayment transactions only so forged total includes multipayment transactions.

Checklist

  • Tests
  • Ready to be merged

@ghost ghost added Complexity: High labels May 25, 2020
@faustbrian
Copy link
Contributor

@rainydio conflicts

@rainydio rainydio marked this pull request as ready for review June 1, 2020 11:02
@faustbrian faustbrian merged commit 3439e52 into develop Jun 1, 2020
@ghost ghost deleted the fix/core-api/multipayment-total-amount branch June 1, 2020 11:11
@ghost ghost removed the Status: Needs Review label Jun 1, 2020
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