rpc, mrc: Fix field name and initialization of mrc_fees_to_staker #2567
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
After testing this fix, it turns out that m_mrc_fees_to_staker is not serialized, because the mrc staker fees can be easily computed by the rules, and in the miner, where the mrc outputs are created, the fees are (implicitly) included in the outputs back to the staker. As part of validation, the mrc fees to the foundation (and therefore the staker) are checked by CheckMRCRewards and then CheckReward. I will add an mrc_fees_to_staker calculation method to compute this. |
jamescowens
force-pushed
the
fix_mrc_fees_to_staker
branch
from
a9195fa
to
4cc3f21
Compare
Implement GetMRCFees in CBlock(). Place output of GetMRCFees() in blocktoJSON conditioned on block.nVersion >= 12.
jamescowens
force-pushed
the
fix_mrc_fees_to_staker
branch
from
4cc3f21
to
1c0b846
Compare
iFoggz
approved these changes
Aug 15, 2022
|
Does it need to be a method of CBlock? I feel like either being a method of a Claim or a static method in src/rpc/blockchain.cpp better as it's just going to be moved out when I move CBlock to src/primitives. |
|
Yes. It doesn’t belong in Claim, because I have to drag in CBlock stuff to do it there. We can put it somewhere else but it is similar to the GetMint.
…Sent from my iPhone
On Aug 15, 2022, at 1:45 PM, div72 ***@***.***> wrote:
Does it need to be a method of CBlock? I feel like either being a method of a Claim or a static method in src/rpc/blockchain.cpp better as it's just going to be moved out when I move CBlock to src/primitives.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were assigned.
|
|
@div72, we will move it out of CBlock along with the other stuff when we refactor to do the primitives split out. |
jamescowens
added a commit
to jamescowens/Gridcoin-Research
that referenced
this pull request
Nov 27, 2022
Added - net: Add and document network messages in protocol.h (backport) gridcoin-community#2533 (@Pythonix) - Define MAX_DIGITS_BTC for magic number in BitcoinUnits::format gridcoin-community#2555 (@barton2526) - rpc: Implementation of getmrcinfo gridcoin-community#2570 (@jamescowens) - init: Add init error message if -printtoconsole and -daemon specified simultaneously gridcoin-community#2571 (@jamescowens) - rpc: getmrcinfo part 2 - add calculated minimum fees and fee boosting and by CPID reporting gridcoin-community#2575 (@jamescowens) - fs: fully initialize `_OVERLAPPED` for win32 gridcoin-community#2587 (@div72) - util: Diagnose Lib Version #1 gridcoin-community#2573 (@MinaFarhan) - util: Implement core diagnostics #2 (@jamescowens) - util: modify Win32LockedPageAllocator to query windows for limit. gridcoin-community#2536 (@div72) - gui, voting: Implement information for wallet holder's votes on poll info cards gridcoin-community#2605 (@jamescowens) Changed - scripted-diff: Drop Darwin version for better maintainability gridcoin-community#2557 (@barton2526) - build: Require gcc8 on Ubuntu Bionic to enable C++17 features gridcoin-community#2579 (@barton2526) - util: Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…) (C++17) gridcoin-community#2564 (@barton2526) - translation: Translation updates gridcoin-community#2581 (@jamescowens) - depends: update urls for dmg tools gridcoin-community#2583 (@div72) - Use ReadLE64 in uint256::GetUint64 instead of duplicating logic gridcoin-community#2586 (@div72) - util: Make Parse{Int,UInt}{32,64} use locale independent std::from_chars(…) (C++17) instead of locale dependent strto{l,ll,ul,ull} gridcoin-community#2592 (@barton2526) - build: don't set PORT=no in config.site gridcoin-community#2593 (@barton2526) - build: Replace `which` command with `command -v` gridcoin-community#2595 (@barton2526) - build: update ax_cxx_compile_stdcxx to serial 14 gridcoin-community#2596 (@barton2526) - gui: Changed the unlocked for staking only icons to green gridcoin-community#2598 (@delta1513) - gui: Translation updates gridcoin-community#2599 (@jamescowens) - build: update CI for linter and actions version gridcoin-community#2606 (@jamescowens) - gui: Update translations gridcoin-community#2608 (@jamescowens) Removed - refactor: remove unused c-string variant of atoi64() gridcoin-community#2562 (@barton2526) - refactor: Remove unused CDataStream::rdbuf method gridcoin-community#2585 (@div72) Fixed - net: Fix some benign races (backport) gridcoin-community#2532 (@Pythonix) - rpc: fix invalid parameter error codes for {sign,verify}message RPCs gridcoin-community#2556 (@barton2526) - build: Fix x86_64 <-> arm64 cross-compiling for macOS gridcoin-community#2560 (@barton2526) - rpc, mrc: Fix field name and initialization of mrc_fees_to_staker gridcoin-community#2567 (@jamescowens) - gui: Add missing resizeTableColumns to fix send address book column widths gridcoin-community#2569 (@jamescowens) - accrual: rebuild snapshot registry on corruption instead of crashing gridcoin-community#2577 (@div72) - doc: Fix link to MurmurHash3.cpp (moved from Google Code to Github) gridcoin-community#2584 (@div72) - fix help text for `revokebeacon` command gridcoin-community#2591 (@Pythonix) - util: Fix spelling error in gridcoinresearchd.cpp gridcoin-community#2590 (@jamescowens) - depends: always use correct ar for win qt build gridcoin-community#2588 (@div72) - util: Fix some bugs due to new implementation and change in BOINC dir handling (@jamescowens) - util: Diagnose lib - Implement changes to solve crash on some Boost 1.66 machines gridcoin-community#2597 (@jamescowens) - contrib: Check for `patch` command, Check for `wget` command gridcoin-community#2594 (@barton2526) - build: Check std::system for -[alert|block|wallet]notify gridcoin-community#2582 (@barton2526) - gui: Changed the wording on the tooltip for the address book gridcoin-community#2602 (@delta1513) - build: pass win32-dll to LT_INIT() gridcoin-community#2601 (@barton2526) - build: minor cleanups to native_clang package gridcoin-community#2600 (@barton2526) - util: restore translations to diagnostics gridcoin-community#2603 (@jamescowens) - refactor: Fix problems found by valgrind gridcoin-community#2607 (@jamescowens)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After looking very carefully at my code as implemented, the m_mrc_fees_to_staker member variable is neither used nor serialized. It was being filled out in the miner, but not serialized. The validation methods in Validator, recompute the mrc fees and check that the fees are correct to the foundation (and therefore also the staker). As such the correct fix for this is to remove the unused member variable entirely.
I have implemented a new method in CBlock, GetMRCFees(), which does the analogous thing for MRC Fees that GetMint() does for the subsidy.
The return values of GetMRCFees() have been move to BlocktoJSON() and are conditioned on block v12 or greater.
Closes #2566.