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

mandatory, voting: Implement poll type validation in protocol #2522

Conversation

jamescowens
Copy link
Member

@jamescowens jamescowens commented Jun 2, 2022

This PR implements poll type validation for poll submission and receipt in protocol. It also implements the display of AVW validation in both RPC and the GUI, fixes the poll ending height by time problem discussed in #2407, and addresses a number of subtle bugs I discovered along the way.

Closes #1788.
Closes #2407.

@jamescowens jamescowens added this to the Kermit's Mom milestone Jun 2, 2022
@jamescowens jamescowens self-assigned this Jun 2, 2022
@jamescowens jamescowens changed the title mandatory, voting: Implement poll type validation mandatory, voting: Implement poll type validation in protocol Jun 2, 2022
@jamescowens jamescowens force-pushed the implement_poll_type_validation branch from 9e0d9f1 to 5e1c576 Compare June 2, 2022 04:30
@iFoggz
Copy link
Member

iFoggz commented Jun 2, 2022

i like where this is going.

@jamescowens jamescowens force-pushed the implement_poll_type_validation branch from bb2f845 to 8626eaf Compare June 4, 2022 14:20
@jamescowens jamescowens force-pushed the implement_poll_type_validation branch 2 times, most recently from 1ff7bca to 0672e91 Compare June 4, 2022 14:53
@jamescowens
Copy link
Member Author

Ok. This is pretty much everything except for the AVW validation. That is going to require some thought and also has to deal with #2407. I will work on that this weekend.

@jamescowens jamescowens force-pushed the implement_poll_type_validation branch from 80f715d to f70a180 Compare June 5, 2022 01:49
@jamescowens jamescowens marked this pull request as ready for review June 5, 2022 01:51
@jamescowens jamescowens requested a review from div72 June 5, 2022 01:52
@jamescowens
Copy link
Member Author

The only thing left on this is the specific issue raised in #2407. I will think about this over the next day.

This new BlockFinder method solves the determinism problem for
poll ending heights by min time when the block times are not
monotonic.
@jamescowens
Copy link
Member Author

This has now been fully tested on a fork with four nodes past a set pollv3height threshold to test out v3 functionality. As a result of the testing a number of adjustments were made. I have also implemented a poll type label which shows up opposite the poll title for v3+ polls and is also a column in the tabular view. This is suppressed for < v3 polls because all polls are type survey less than v3.

@jamescowens
Copy link
Member Author

jamescowens commented Jun 12, 2022

image
You can see here that v3 polls have the "validated" moniker as soon as they have reached the AVW % threshold required by the poll type rule. They also have the poll type label. There is one older v2 poll listed there that does not.

The moniker or tag for validation has three states:

  1. Blank for v2 polls or v3 polls that have not reached vote weight % AVW threshold for validation and have not expired (are still active)
  2. Green "Validated" for v3 polls that have reached vote weight % AVW threshold for validation. This will appear immediately when that has been reached, regardless of whether the poll is expired or not.
  3. Red "Invalid" for v3 polls that have expired and failed to reach the vote weight % AVW threshold.

@jamescowens jamescowens force-pushed the implement_poll_type_validation branch 3 times, most recently from 16a8749 to 269fcf3 Compare June 13, 2022 01:32
Also add required field rules

This is the baseline commit for this functionality. It does not actually
implement the injection of required fields, nor the validation of such,
except for the basic WellFormed check.

Note this is patterned off of the Choices class for consistency.
@jamescowens jamescowens force-pushed the implement_poll_type_validation branch from 269fcf3 to a8190bf Compare June 13, 2022 01:56
@jamescowens jamescowens force-pushed the implement_poll_type_validation branch 2 times, most recently from c4f06fe to 406638e Compare June 13, 2022 19:52
@jamescowens
Copy link
Member Author

Test of creation of projects poll with required fields:

16:11:06
addpoll project "Dummy Project 1 whitelist request" 21 "Should we whitelist this project?" "" 2 1 "http://gridcoin.us" "project_name=Dummy Project 1;project_url=https://dummy1.com"

16:11:06
{
"title": "Dummy Project 1 whitelist request",
"id": "723053b76b4fe40c8ee44ed013dcb62bbd2226da67d0555e2593c0896753da74",
"question": "Should we whitelist this project?",
"url": "http://gridcoin.us",
"additional_fields": [
{
"project_name": {
"value": "Dummy Project 1",
"required": true
}
},
{
"project_url": {
"value": "https://dummy1.com",
"required": true
}
}
],
"poll_type": "Project Listing",
"poll_type_id": 2,
"weight_type": "Magnitude+Balance",
"weight_type_id": 3,
"response_type": "Yes/No/Abstain",
"response_type_id": 1,
"duration_days": 21,
"expiration": "07-04-2022 20:11:06",
"timestamp": "06-13-2022 20:11:06",
"choices": [
{
"id": 0,
"label": "Yes"
},
{
"id": 1,
"label": "No"
},
{
"id": 2,
"label": "Abstain"
}
]
}

16:15:37
listpolls

16:15:37
[
{
"title": "Dummy Project 1 whitelist request",
"id": "723053b76b4fe40c8ee44ed013dcb62bbd2226da67d0555e2593c0896753da74",
"question": "Should we whitelist this project?",
"url": "http://gridcoin.us",
"additional_fields": [
{
"project_name": {
"value": "Dummy Project 1",
"required": true
}
},
{
"project_url": {
"value": "https://dummy1.com",
"required": true
}
}
],
"poll_type": "Project Listing",
"poll_type_id": 2,
"weight_type": "Magnitude+Balance",
"weight_type_id": 3,
"response_type": "Yes/No/Abstain",
"response_type_id": 1,
"duration_days": 21,
"expiration": "07-04-2022 20:11:06",
"timestamp": "06-13-2022 20:11:06",
"choices": [
{
"id": 0,
"label": "Yes"
},
{
"id": 1,
"label": "No"
},
{
"id": 2,
"label": "Abstain"
}
],
"votes": 0
},
{
"title": "Testnet build frequency",
"id": "996896739981aca390d0f06aa05422292aa8510778f10295897e72f9e33a5a49",
"question": "Are testnet builds frequent enough?",
"url": "http://gridcoin.us",
"additional_fields": [
],
"poll_type": "Community",
"poll_type_id": 7,
"weight_type": "Magnitude+Balance",
"weight_type_id": 3,
"response_type": "Yes/No/Abstain",
"response_type_id": 1,
"duration_days": 21,
"expiration": "07-03-2022 06:04:05",
"timestamp": "06-12-2022 06:04:05",
"choices": [
{
"id": 0,
"label": "Yes"
},
{
"id": 1,
"label": "No"
},
{
"id": 2,
"label": "Abstain"
}
],
"votes": 4
},
{
"title": "Testnet build quality",
"id": "ccf94e0120c9ca67b45560d726e6c17b944b71d86da62177167ba25c31494483",
"question": "Are the testnet builds at a sufficiently polished level?",
"url": "http://gridcoin.us",
"additional_fields": [
],
"poll_type": "Community",
"poll_type_id": 7,
"weight_type": "Magnitude+Balance",
"weight_type_id": 3,
"response_type": "Yes/No/Abstain",
"response_type_id": 1,
"duration_days": 21,
"expiration": "07-03-2022 06:10:38",
"timestamp": "06-12-2022 06:10:38",
"choices": [
{
"id": 0,
"label": "Yes"
},
{
"id": 1,
"label": "No"
},
{
"id": 2,
"label": "Abstain"
}
],
"votes": 4
},
{
"title": "Testnet participation",
"id": "31d6e2bbbb7f7f98dac152fc8338440139ff587e4f1b683522b9887d1e2b41f2",
"question": "Are there enough testnet participants?",
"url": "http://gridcoin.us",
"additional_fields": [
],
"poll_type": "Survey",
"poll_type_id": 1,
"weight_type": "Magnitude+Balance",
"weight_type_id": 3,
"response_type": "Yes/No/Abstain",
"response_type_id": 1,
"duration_days": 21,
"expiration": "06-25-2022 08:25:44",
"timestamp": "06-04-2022 08:25:44",
"choices": [
{
"id": 0,
"label": "Yes"
},
{
"id": 1,
"label": "No"
},
{
"id": 2,
"label": "Abstain"
}
],
"votes": 6
}
]

@jamescowens
Copy link
Member Author

image

@jamescowens
Copy link
Member Author

Now I have to create an additional fields model and table view to be able to deal with these in the GUI.

This also makes some modifications to PollBuilder.
@jamescowens jamescowens force-pushed the implement_poll_type_validation branch from 406638e to ef4ec55 Compare June 14, 2022 17:08
@jamescowens
Copy link
Member Author

@div72 I want to cut this off and get this into testnet. I will do the GUI part for the additional fields, which is not consensus critical, in a separate PR.

@jamescowens
Copy link
Member Author

After you approve, I will add one more commit for the testnet blockheight where v3 polls are going to go live.

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.

Looks pretty good otherwise. I'll commence testing.

src/gridcoin/voting/payloads.h Outdated Show resolved Hide resolved
src/rpc/voting.cpp Outdated Show resolved Hide resolved
src/gridcoin/voting/poll.h Show resolved Hide resolved
src/gridcoin/voting/poll.cpp Show resolved Hide resolved
…onalFields

This adds the list level WellFormed() check to the PollBuilder AddAdditionalFields
check primarily to check and ensure that the m_required boolean is set correctly
in accordance with the poll type rules.
@jamescowens
Copy link
Member Author

@div72 Please give this one last look. I think it is ready to merge. Then I will rebase #2525 and we will get the GUI part out.

@jamescowens jamescowens merged commit d6e5579 into gridcoin-community:development Jun 26, 2022
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)
@jamescowens jamescowens deleted the implement_poll_type_validation branch November 4, 2022 23:19
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.

BlockFinder::FindByMinTime May Retroactively Change Computed AVW Feature Request: Poll categories.
3 participants