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

Write app call addresses in txn_participation table #770

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

tolikzinovyev
Copy link
Contributor

Summary

Closes #768.

TODO: don't filter out the zero address. It is a real address and people are curious about how much algos it receives.

Test Plan

Added a unit test that checks that app call address are written in the txn_participation table.

@tolikzinovyev tolikzinovyev force-pushed the tolik/participation branch 2 times, most recently from 2264ab9 to eda1a3a Compare November 3, 2021 19:33
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

One request otherwise looks fine.

db, shutdownFunc := setupPostgres(t)
defer shutdownFunc()

block := bookkeeping.Block{
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can use block, err := test.MakeBlockForTxns(test.MakeGenesisBlock().BlockHeader, &stxnad) instead of constructing this object yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to use a round number that is not 0 or 1 for the basic test, and now that the tests are merged, I think we should keep the current construct.

@@ -410,6 +410,70 @@ func TestWriterTxnParticipationTableBasic(t *testing.T) {
}
}

func TestWriterTxnParticipationTableAppCallAddresses(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you merge this with TestWriterTxnParticipationTableBasic? These tests appear to be identical except for the transactions and expected results, so they could be data-driven with one of those test case structs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2021

Codecov Report

Merging #770 (9e30c96) into develop (9833f05) will decrease coverage by 0.31%.
The diff coverage is 31.25%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #770      +/-   ##
===========================================
- Coverage    54.78%   54.47%   -0.32%     
===========================================
  Files           27       28       +1     
  Lines         3875     3870       -5     
===========================================
- Hits          2123     2108      -15     
- Misses        1471     1481      +10     
  Partials       281      281              
Impacted Files Coverage Δ
accounting/accounting.go 0.00% <0.00%> (ø)
idb/postgres/internal/writer/writer.go 77.06% <100.00%> (-1.01%) ⬇️
idb/postgres/postgres.go 47.58% <100.00%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9833f05...9e30c96. Read the comment docs.

Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

Thanks for the change, looks good.

@tolikzinovyev tolikzinovyev merged commit 7dff12e into develop Nov 4, 2021
@tolikzinovyev tolikzinovyev deleted the tolik/participation branch November 4, 2021 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add application call accounts to txn_participation table
3 participants