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

Part 2 of Issue246: Store the validator stake in the database #2457

Merged
merged 3 commits into from Sep 7, 2021

Conversation

hewison-chris
Copy link
Contributor

This PR is to prepare some things to facilitate paying out the fees and rewards at the end of the next payment cycle when UTXO set may no longer have our inputs which we need to get the values from.

Make separate check and message for Coinbase or already in the tx pool.
The documentation was also updated to be correct.
`Fee` is an `Amount` which has an underlying value type of `ulong`,
this can be stored as an INTEGER in the `SQLite3` database.
For convenience when the amount that was staked for a validator is
needed.
@hewison-chris hewison-chris linked an issue Sep 7, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #2457 (a945c81) into v0.x.x (a449f44) will decrease coverage by 0.09%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           v0.x.x    #2457      +/-   ##
==========================================
- Coverage   87.23%   87.13%   -0.10%     
==========================================
  Files         154      154              
  Lines       14779    14770       -9     
==========================================
- Hits        12892    12870      -22     
- Misses       1887     1900      +13     
Flag Coverage Δ
integration 67.02% <ø> (+28.98%) ⬆️
unittests 86.93% <96.15%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
source/agora/consensus/EnrollmentManager.d 96.63% <ø> (-0.01%) ⬇️
source/agora/consensus/data/ValidatorInfo.d 100.00% <ø> (ø)
source/agora/consensus/Ledger.d 94.10% <80.00%> (-0.14%) ⬇️
source/agora/consensus/Fee.d 96.25% <100.00%> (+0.03%) ⬆️
source/agora/consensus/state/ValidatorSet.d 87.57% <100.00%> (+0.26%) ⬆️
source/agora/consensus/validation/Enrollment.d 100.00% <100.00%> (ø)
source/agora/utils/Log.d 54.47% <0.00%> (-11.95%) ⬇️
source/agora/consensus/protocol/Nominator.d 87.99% <0.00%> (+0.46%) ⬆️

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 a449f44...a945c81. Read the comment docs.

@hewison-chris hewison-chris changed the title Part of Issue246: Store the validator stake in the database Part 2 of Issue246: Store the validator stake in the database Sep 7, 2021
@@ -388,7 +388,7 @@ public class FeeManager

this.db.execute(
"REPLACE INTO accumulated_fees (fee, public_key) VALUES (?,?)",
this.accumulated_fees[stake.output.address], stake.output.address);
this.accumulated_fees[stake.output.address].tupleof[0], stake.output.address);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need the tupleof though ? Since there's a toString overload it should pick that up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it uses INTEGER SQL type now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, it only uses them for string type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If not needed for Amount -> INTEGER I will remove in next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact it is removed already as I had to change the table again to store for each height.

Comment on lines +78 to +79
stake = utxo_set_value.output.value;

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, isInvalidReason should not provide the stake to the caller.

Copy link
Collaborator

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

The out parameter is problematic, but I have a commit somewhere that would solve this, so let's move ahead with this.

@Geod24 Geod24 merged commit 834ed0f into bosagora:v0.x.x Sep 7, 2021
@hewison-chris hewison-chris deleted the issue246_block_reward_2 branch September 7, 2021 05:56
@@ -388,7 +388,7 @@ public class FeeManager

this.db.execute(
"REPLACE INTO accumulated_fees (fee, public_key) VALUES (?,?)",
this.accumulated_fees[stake.output.address], stake.output.address);
this.accumulated_fees[stake.output.address].tupleof[0], stake.output.address);
Copy link
Contributor

@linked0 linked0 Sep 7, 2021

Choose a reason for hiding this comment

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

this.accumulated_fees[stake.output.address].tupleof[0], stake.output.address);
Out of curiosity, what's the meaning of tupleof in this context?
I only found the meaning for that in this page.

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.

Define and implement block rewards
3 participants