-
Notifications
You must be signed in to change notification settings - Fork 172
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Gridcoin Research 3.5.9.1/MSI=42.5 Leisure Upgrade - Voting system security enhancement - Neural Network upgrade (allows us to come to a consensus, reduces ddos on project servers)
- Loading branch information
Showing
10 changed files
with
288 additions
and
99 deletions.
There are no files selected for viewing
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
15 changes: 9 additions & 6 deletions
15
contrib/Installer/GridcoinRDTestHarness/GridcoinRDTestHarness/Form1.vb
Large diffs are not rendered by default.
Oops, something went wrong.
11 changes: 11 additions & 0 deletions
11
...inRDTestHarness/GridcoinRDTestHarness/bin/Debug/GridcoinRDTestHarness.vshost.exe.manifest
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <?xml version="1.0" encoding="UTF-8" standalone="yes"?> | ||
| <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0"> | ||
| <assemblyIdentity version="1.0.0.0" name="MyApplication.app"/> | ||
| <trustInfo xmlns="urn:schemas-microsoft-com:asm.v2"> | ||
| <security> | ||
| <requestedPrivileges xmlns="urn:schemas-microsoft-com:asm.v3"> | ||
| <requestedExecutionLevel level="asInvoker" uiAccess="false"/> | ||
| </requestedPrivileges> | ||
| </security> | ||
| </trustInfo> | ||
| </assembly> |
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
Oops, something went wrong.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, now only you can delete polls and silence voters.
Rest of CPP seems good.
Haven't checked the VB.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well actually I didnt do that intentionally, I was thinking of 'preventing' delete in the same manner as preventing deletion of a boinc project, but just for the record, I have never deleted a poll yet (not even an accidental one created by CM), and do not intend to- you can write a program to monitor the polls to see if any ever get deleted, as I dont plan on it.
But yet now that you mention it, we can keep this in mind, and create an issue to address every flaw in the voting system, including me having the power to delete a poll.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean it in that way, I trust you. Now not everyone can delete polls and silence voters.
I don't see a problem with deleting a poll by the pool creator or admin. For the exact situation of posting polls by accident.
On the other hand, votes should never be deleted.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the reversion to a centralized stats source intended to be a temporary thing, or permanent?
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nateonthenet , it's more of a reversion to a centralized cache with primary project server header etags- and failover ability to download from the project servers (the project servers are still used for the headers, and for failing over when a file cant be downloaded). No, its temporary, to buy us time until we code in the ability for nodes to share GZ files amongst each other inside the neural network. We are trying to prevent ddosing the project sites.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@3ullShark , Sorry to hear that. When I tested it on my test nodes, I saw whitelisted projects, and mags being populated for the list of cpids, but did not realize only 7 projects were being included. I will have to check my logs for similar behavior. Will check it out today.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so our new superblock 950458 was staked with 3591, interesting. Im not able to reproduce the same problem-my logs from yesterday are clean. So today I erase the neural network, verified the 25 projects in the whitelist are sent in to the neural network (pass), resync execute syncdpor2, pass, and here is something important, note that cpid 7d0d* he is the one on top of the list with a mag of 21833, he is a good guinea pig to look at, as he is boincing all projects. If you drill into his row, he is a good candidate to count the project count - in my case, I pre-deleted all project* files before resyncing. Then drilled into 7d0d* by double clicking - and I see he is boincing all 24 projects - so in my case the superblock would have included 24 projects and 2200 cpids. BUT thats just my node in the US....
So I think what you might be saying is we might have an internationalization (locale) issue here that I am not affected by. Bullshark, or Marco, what happens when you go into neuralnetwork folder, and delete all files starting with Project* and you re-sync? How many projects do you see in the drill in for 7d0d after doing so?
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anyone can trap the invalid date value, and log it that would help us fix this issue, please before doing so delete everything in the neural network and start from scratch. Maybe its a UTC date in the future or possibly a date with a unix timestamp below zero, or could be a string formatting issue with a timezone at the end that isnt recognized?
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are to release another version we should update the stat URL for Rosetta, right?
@gridcoin It synced just fine for me, 24 projects but it complains about the dat files being in use by another process a couple of times.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@3ullShark thanks, I think I found it based on the log you emailed in- its precarious because the log alludes to the problem being in updMagnitudePhase1, but in reality the 'dateTime' parameter gave it away as to being in the GetRowAgeInMins area .
It looks like the problem is the exp_avg rac time is already UTC. I am trying to convert that timestamp (a unix timestamp) over to a local datetime, then over to UTC. Recently on Jul2, the Sahara desert had a timezone change and that means one of the dates in the 8th project were trying to land inside the timezone that does not exist anymore.
So the change required is I will change the UnixTimestampToDate to return the UTC time, but yet where we convert the Etag timestamp (that has a full datetime string with timezone in it) still needs the timezone conversion - but I will make a resilient function for that in case the Etag string ever has something in it that does not convert to UTC - in that function Ill back the clock back 2 hours and return something valid. The main thing is I am trying to ensure both calls return a consensus globally so the NN can function properly globally. Ill retest this now.
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be able to get away with always using UTC unless the time gets contaminated with a local time, such as the creation time of a file in the local filesystem, assuming Windows uses localtime on files..
fa9cf95There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok cool, I will change it to Last-Modified, and yes now we use UTC in both places. Starting yesterday there should be no reliance on the timestamps from the filesystem (it always uses the etag Last-Modified for the UTC timestamp to invalidate itself, and the only other thing is the RAC filtering by UTC timestamp to filter out the RAC rows that are over 32 days old).
Will re-test with Last-Modified.