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

gui: Initial implementation of GUI MRC submission form #2513

Merged
merged 21 commits into from
May 21, 2022

Conversation

jamescowens
Copy link
Member

@jamescowens jamescowens commented May 9, 2022

This is the initial implementation of the GUI submission form, implemented mainly in mrcmodel.h/cpp and mrcrequestpage.h/cpp/ui. Note that it is from my mrc_wizard branch, which in turn is based on the mrc branch, so this PR will show the mrc branch commits until the latest mrc PR is merged and the mrc_wizard branch is rebased [done].

As of 5/16/2022 this is a fully functional MRC submission screen. It is currently accessed by a tool button next to the researcher "gear" button on the main screen that is visible ONLY if the wallet is in solo mode, the researcher has a valid beacon, and the wallet is in sync.

The screen looks like this:
image
(This picture shows the screen in the state where an error is displayed that it is too soon since the last payment, as a tooltip.)

The PR touches several other areas to provide minor extensions, and in the case of the rpc function createmrcrequests, bug fixes on the queue head/tail calculation, and implementation of the pay limit fee calculation, which in the case of where the queue is oversubscribed (i.e. the number of MRCs in the memory pool exceeds the maximum that can be paid out in a block), provides the fee paid by the last MRC in the queue sorted by fee (which is the last one that will be paid by the miner). This is the minimum hurdle to get over when fee boosting to displace the last entry in the pay limit and take over the last slot.

The model and page have implemented full signaling from the core of state changes that affect MRC status to allow automatic updating of the screen without having to push the "update" button. (In fact, the update button may be removed soon, as it is probably not necessary anymore.) Note this also has the advantage that the MRC request page responds correctly to actions taken by the local node core (or other nodes) as they come across the network. For example if the local node owner has the MRC request screen open and decides to submit an MRC via the debug console or rpc createmrcrequest, if the request is successful, the MRC request screen will reflect the submission in real time as if the MRC submit screen were used.

If an MRC cannot be submitted, the triangular error icon will be displayed (with an appropriate tooltip) and the submit button will be disabled. The error conditions can be seen very easily by looking at MRCModel::refresh() and MRCModel::submitMRC().

See in MRCModel.h

enum class MRCRequestStatus
{
NONE,
ELIGIBLE,
NOT_VALID_RESEARCHER,
CREATE_ERROR,
PENDING,
QUEUE_FULL,
PENDING_CANCEL,
STALE_CANCEL,
ZERO_PAYOUT,
EXCESSIVE_FEE,
WALLET_LOCKED,
SUBMIT_ERROR
};

also see

enum ModelStatus {
    VALID,
    NOT_VALID_RESEARCHER,
    INVALID_BLOCK_VERSION,
    OUT_OF_SYNC,
    NO_BLOCK_UPDATE_FROM_INIT
};

So everything except for NONE and ELIGIBLE in MRCRequestStatus is considered an error state. The handling of "PENDING", however, is specialized, as this means that a new MRC cannot be submitted while there is an existing MRC already (submitted) in the memory pool.

The handling of QUEUE_FULL, PENDING_CANCEL, and STALE_CANCEL are also specialized and represent special handling for corner cases given the MRC submission and payment model implemented in protocol. In the case of QUEUE_FULL, this represents where the number of MRCs in the memory pool is at the output (pay) limit, in which case the submit button is disabled and an error triangle appears suggesting that the user boost the fees if desired to try to slot in and push another MRC down. The PENDING_CANCEL represents the inverse condition from that just described, which is that the user had successfully submitted an MRC but it was pushed down in priority by another submission at a higher fee level, and there is past the pay limit and will not be paid by the staker. The STALE_CANCEL is the end stage of PENDING_CANCEL, where an unpaid MRC remains in the memory pool, but refers to a block that is no longer at the head of the chain. The tooltip for all of these error conditions has helpful commentary for the user.

Note that the "ELIGIBLE" state is the only state where the submit button is enabled.

For the ModelStatus, this controls whether the MRC status and submission widgets are displayed or hidden behind a blank screen with an information message that MRC submissions are not available. The MRC status and submission widgets are only shown when the ModelStatus is VALID. All other states result in a cover screen and an appropriate information message.

Here is a screenshot of the normal state with the wallet unlocked, where the minimum time since the last payment has been met, and the queue is empty:
image

After submission on a full queue where the node took priority and displaced someone else due to a higher fee:
image

Notice that the submit button is disabled because there is a pending MRC. There is a green check box to indicate the MRC submission was successful.

This closes gridcoin-community/Gridcoin-Tasks#218.

@jamescowens jamescowens self-assigned this May 9, 2022
@jamescowens jamescowens added this to the Kermit's Mom milestone May 9, 2022
@jamescowens jamescowens marked this pull request as draft May 9, 2022 22:46
@jamescowens jamescowens force-pushed the mrc_wizard branch 11 times, most recently from 2e76211 to f6257ab Compare May 13, 2022 19:17
@jamescowens jamescowens changed the title gui: Tracking PR for implementation of GUI MRC submission form (WIP) gui: Initial implementation of GUI MRC submission form May 14, 2022
@jamescowens jamescowens force-pushed the mrc_wizard branch 3 times, most recently from 68d7a76 to 3ed89f4 Compare May 14, 2022 22:05
@jamescowens jamescowens marked this pull request as ready for review May 14, 2022 23:38
@jamescowens jamescowens force-pushed the mrc_wizard branch 9 times, most recently from 8686a86 to 2280ef6 Compare May 15, 2022 20:18
Also implement cover wait page for MRC screen when MRC submission is
impossible.
A successfully submitted MRC can be subsequently pushed down in the
queue past the pay limit by other MRCs submitted with higher fees.
This means that the submitted MRC should revert to QUEUE_FULL status.

This also tweaks the WALLET_LOCKED status to only apply if an MRC
is not "found", i.e. there is no submitted MRC present. This is the
correct behavior, because the wallet had to be unlocked to submit
the MRC, and this would represent the situation where the wallet
was relocked after the MRC was submitted. You can't submit a new
MRC until after the existing submitted MRC is either paid or deleted,
so the WALLET_LOCKED status should only apply when there is no MRC
with the walletholder's CPID in the queue.
Also fix dangling mrc request page when exit called from main screen
while mrc request dialog is open.
Also removal of a now unnecessary debugging LogPrintf.
src/qt/mrcmodel.cpp Outdated Show resolved Hide resolved
Co-authored-by: div72 <60045611+div72@users.noreply.github.com>
@jamescowens jamescowens requested a review from div72 May 20, 2022 21:48
@jamescowens
Copy link
Member Author

@div72 I applied the one suggestion on the string handling. Anything else of note on this?

@div72
Copy link
Member

div72 commented May 21, 2022

@jamescowens The signaling from numBlocksChanged on mrcmodel.cpp#L63 seems a bit unnecessary, and I have a couple of nits but nothing major.

I am waiting for my testnet beacon to get verified, but I can give a code review ACK if @denravonska can give a tested ACK.

@denravonska
Copy link
Member

Test ACK on testnet:

  • Claim not possible when out of sync
  • Claim not possible without a beacon
  • Claim not possible if one is pending
  • Claim not possible if too soon from a previous one
  • Claim possible if eligible
  • Claim is paid out as expected
  • UI updates when conditions are updated (IIRC)

Not tested priority behavior as that had been tested by @jamescowens already.

Copy link
Member

@div72 div72 left a comment

Choose a reason for hiding this comment

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

utACK.

@jamescowens jamescowens merged commit a9f8b0f into gridcoin-community:development May 21, 2022
@jamescowens jamescowens deleted the mrc_wizard branch June 26, 2022 05:06
jamescowens added a commit to jamescowens/Gridcoin-Research that referenced this pull request Jul 31, 2022
Added
 - test: Add TrimString(...) tests gridcoin-community#2447 (@barton2526)
 - test: Add dead code detection gridcoin-community#2449 (@barton2526)
 - test: Add explicit references to related CVE's in comments gridcoin-community#2467 (@barton2526)
 - test: Add testing of ParseInt/ParseUInt edge cases with leading +/-/0:s gridcoin-community#2470 (@barton2526)
 - consensus, contract, mining, researcher, rpc, staking, gui: Implementation of MRC - baseline functionality gridcoin-community#2425 (@jamescowens)
 - consensus: MRC mandatory implementation code gridcoin-community#2471 (@jamescowens)
 - test: Add upstream sync_tests.cpp gridcoin-community#2481 (@barton2526)
 - net: Countermeasures against eclipse attacks gridcoin-community#2454 (@Pythonix)
 - lint: add script to check for https violations gridcoin-community#2491 (@div72)
 - util: Add flatpath BOINC data directory path resolution for Linux gridcoin-community#2499 (@jamescowens)
 - gui: Add beaconExpired() to researchermodel gridcoin-community#2498 (@jamescowens)
 - consensus: Add missing block nVersion check for v12 blocks in AcceptBlock gridcoin-community#2502 (@jamescowens)
 - gui, util: Add AccrualChangedFromStakeOrMRC core signal gridcoin-community#2503 (@jamescowens)
 - util: Change default -dbcache to 100 MB and also implement -txindexdbcache gridcoin-community#2507 (@jamescowens)
 - rpc, util, consensus: Implement exception handling framework for MRC and fix ValidateMRC to deal with testnet consensus issue gridcoin-community#2508 (@jamescowens)
 - gui: Initial implementation of GUI MRC submission form gridcoin-community#2513 (@jamescowens)
 - build: Port over Bitcoin's translation docs gridcoin-community#2439 (@jamescowens)
 - (2/3) build: integrate libsecp256k1 gridcoin-community#2492 (@div72)
 - gui: New MRC request icon gridcoin-community#2526 (@jamescowens)
 - mandatory, voting: Implement poll type validation in protocol gridcoin-community#2522 (@jamescowens)
 - gui, voting: Implement poll additional fields gui components gridcoin-community#2525 (@jamescowens)
 - gui, researcher: Add GDPR protection display gridcoin-community#2527 (@jamescowens)
 - consensus, rpc: Kermit's mom hardfork (2671700) gridcoin-community#2551 (@jamescowens)

Changed
 - net: Hard Coded Seed Node Cleanup gridcoin-community#2427 (@barton2526)
 - script: Add More Generated Files to Gitignore gridcoin-community#2435 (@RoboticMind)
 - gui: Update copyright year to 2022 for Gridcoin About dialog box gridcoin-community#2443 (@jamescowens)
 - rpc: Change type field in ListTransactions to lower case gridcoin-community#2441 (@jamescowens)
 - refactor: Replace memset calls with array initialization gridcoin-community#2452 (@barton2526)
 - refactor: Changed some parameters from pass by value to pass by reference gridcoin-community#2455 (@Pythonix)
 - ci, cd: improve caching gridcoin-community#2461 (@div72)
 - contrib: port recent macdeployqtplus changes gridcoin-community#2465 (@div72)
 - test: Test for expected return values when calling functions returning a success code gridcoin-community#2464 (@barton2526)
 - build: Improve error message when pkg-config is not installed gridcoin-community#2460 (@barton2526)
 - test: Bump shellcheck, mypy versions gridcoin-community#2463 (@barton2526)
 - build: Update depends packages (expat, fontconfig, freetype, libXau, libxcb, xcb_proto, xproto) gridcoin-community#2466 (@barton2526)
 - lint: run mypy over contrib/devtools gridcoin-community#2475 (@barton2526)
 - build, lint: Remove x-prefix's from comparisons, Fix some shell script issues the linter complains about, Re-enable boost include checks gridcoin-community#2478 (@barton2526)
 - test: Avoid copies of CTransaction gridcoin-community#2479 (@barton2526)
 - ci: change windows CI to Focal, modify wrap_wine to use wine64 for 64bit binaries gridcoin-community#2484 (@barton2526)
 - build: Qt 5.15.2 gridcoin-community#2486 (@barton2526)
 - net: No longer send local address in addrMe gridcoin-community#2459 (@Pythonix)
 - voting, gui, rpc: Enhance PollResult and AVW calculation to improve pool handling gridcoin-community#2489 (@jamescowens)
 - (1/3) refactor: port some misc changes from upstream gridcoin-community#2485 (@div72)
 - build: Try posix-specific CXX first for mingw32 host, Fix Windows cross-compiling with Qt 5.15 gridcoin-community#2494 (@barton2526)
 - Improve upon scanforunspent rpc gridcoin-community#2468 (@iFoggz)
 - rpc: Change tail_fee and head_fee to display in GRC rather than Halfords in createmrcrequest gridcoin-community#2501 (@jamescowens)
 - scripted-diff: change http to https in copyright text gridcoin-community#2504 (@div72)
 - qt, refactor: Use enum type as switch argument in *TableModel gridcoin-community#2496 (@barton2526)
 - build, qt: bump Qt5 version to 5.15.3 gridcoin-community#2510 (@barton2526)
 - utils: run commands using utf-8 string on Windows gridcoin-community#2514 (@barton2526)
 - prevector: enforce is_trivially_copyable_v gridcoin-community#2516 (@div72)
 - crypto: Unroll the ChaCha20 inner loop for performance gridcoin-community#2515 (@div72)
 - gui: Modify VerifyTCPPort to use the status of CheckOutboundConnectionCount gridcoin-community#2506 (@jamescowens)
 - gui: Fix transaction history table column size behavior gridcoin-community#2520 (@jamescowens)
 - log: Use consistent wording in random.cpp log gridcoin-community#2538 (@div72)
 - lint: Use newer versions of our lint packages, remove yq gridcoin-community#2541 (@barton2526)
 - (1/2) validation: move CBlock validation methods to validation.cpp gridcoin-community#2539 (@div72)
 - gui: Implement proportional column resizing for Addressbook with memory gridcoin-community#2543 (@jamescowens)
 - build: remove redundant warning flags gridcoin-community#2546 (@barton2526)
 - qt: Prefix makefile variables with QT_ gridcoin-community#2547 (@barton2526)
 - build: remove build stubs for external leveldb gridcoin-community#2550 (@barton2526)
 - build, refactor: Improve package version usage gridcoin-community#2549 (@barton2526)
 - build: minor boost tidyups gridcoin-community#2548 (@barton2526)

Removed
 - rpc, util: Remove caching from BlockFinder gridcoin-community#2490 (@jamescowens)
 - test: remove obsolete check sig test gridcoin-community#2552 (@div72)

Fixed
 - build: fix unoptimized libraries in depends gridcoin-community#2428 (@barton2526)
 - build: don't use deprecated brew package names gridcoin-community#2429 (@barton2526)
 - qt: fix shutdown on MacOS gridcoin-community#2440 (@div72)
 - net: Do not add random inbound peers to addrman gridcoin-community#2451 (@barton2526)
 - util: skip trying to set the locale on NetBSD gridcoin-community#2448 (@barton2526)
 - build: change bundle id gridcoin-community#2462 (@div72)
 - net: Do not propagate obviously poor addresses onto the network gridcoin-community#2453 (@Pythonix)
 - ci: Fix CI build title to reflect that we are building for bionic, not xenial gridcoin-community#2469 (@barton2526)
 - lint: Fix misc typos gridcoin-community#2472 (@barton2526)
 - util: Fix crash when parsing command line with -noincludeconf=0, Properly handle -noincludeconf on command line gridcoin-community#2473 (@barton2526)
 - build: Fix several minor linter errors gridcoin-community#2476 (@jamescowens)
 - tests: Don't access out of bounds array index: array[sizeof(array)] gridcoin-community#2480 (@barton2526)
 - script: Fix and Minify Icon SVG gridcoin-community#2488 (@RoboticMind)
 - gui: Add missing null pointer check for m_beacon gridcoin-community#2500 (@jamescowens)
 - util: Fix BN_zero macro in key.cpp for OpenSSL 3.0 gridcoin-community#2497 (@jamescowens)
 - rpc, contract: Adjust ValidateMRC, CreateMRC, and createmrcrequest to correct provided fee handling gridcoin-community#2505 (@jamescowens)
 - consensus: Move DoS into contract validators to allow variability of DoS based on context and further fixes to ValidateMRC gridcoin-community#2512 (@jamescowens)
 - lockedpool: When possible, use madvise to avoid including sensitive information in core dumps gridcoin-community#2509 (@barton2526)
 - refactor: Fix some minor linter complaints gridcoin-community#2517 (@jamescowens)
 - miner: Miner Logger bug fix gridcoin-community#2518 (@iFoggz)
 - key: properly parse short DER private keys gridcoin-community#2519 (@div72)
 - researcher: Fix ReadClientStateXml() crash on wrong BOINC directory permissions gridcoin-community#2524 (@jamescowens)
 - init: fix daemon forking gridcoin-community#2521 (@div72)
 - scraper: Change open mode from append to truncate for auth file gridcoin-community#2528 (@jamescowens)
 - gui: New mrc contract icon try #2 gridcoin-community#2529 (@jamescowens)
 - gui: Remove white outlines on MRC icon gridcoin-community#2530 (@a123b)
 - voting: Change m_additional_fields serialization gridcoin-community#2531 (@jamescowens)
 - build, qt: Fix `QMAKE_CXXFLAGS` expression for `mingw32` host gridcoin-community#2537 (@div72)
 - logging: fix logging empty thread name gridcoin-community#2535 (@div72)
 - trivial: fix comment in account header guard gridcoin-community#2542 (@div72)
 - build: Restrict check for CRC32C intrinsic to aarch64 gridcoin-community#2544 (@barton2526)
 - build: force CRCCheck in Windows installer gridcoin-community#2545 (@barton2526)
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.

Manual Reward Claim (MRC)
3 participants