Skip to content

[INVALID] Custom mining with verification. #360

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

Closed

Conversation

cyber-pc
Copy link
Collaborator

@cyber-pc cyber-pc commented Apr 5, 2025

This PR contains,

  • Save/Load the custom mining with state save/load.
  • Only recording the verified solution that submit by the node's OPERATOR

@cyber-pc cyber-pc requested a review from krypdkat April 5, 2025 08:24
@cyber-pc cyber-pc added the invalid This doesn't seem right label Apr 5, 2025
static constexpr unsigned long long _invalidTaskIndex = 0xFFFFFFFFFFFFFFFFULL;
void init()
{
allocatePool(CUSTOM_MINING_TASK_STORAGE_COUNT * sizeof(CustomMiningTask), (void**)&_customMiningTasks);
Copy link
Contributor

Choose a reason for hiding this comment

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

please use allocPoolWithErrorLog instead

_customMiningPhaseCount = 1;
}

void checkAndReset()
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a note here which process calling this

}

// Binary search for taskIndex or closest greater-than task index
unsigned long long searchTaskIndex(unsigned long long taskIndex, bool& exactMatch) const
Copy link
Contributor

Choose a reason for hiding this comment

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

please add unit tests for this function, we usually have hang/bug on binary search function before

static constexpr unsigned long long _invalidIndex = 0xFFFFFFFFFFFFFFFFULL;
void init()
{
allocatePool(maxItems * sizeof(DataType), (void**)&_data);
Copy link
Contributor

Choose a reason for hiding this comment

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

please use allocPoolWithErrorLog instead

struct RespondCustomMiningTask
{
static constexpr int _maxNumberOfTasks = 4;
static constexpr int _maxNumberSolutions = 64;
Copy link
Contributor

Choose a reason for hiding this comment

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

any intention behind these numbers 4 and 64?

@@ -2,7 +2,7 @@

#include <lib/platform_efi/uefi.h>

static unsigned char consoleLoggingLevel = 1;
static unsigned char consoleLoggingLevel = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert this

@@ -4,7 +4,7 @@

// Do NOT share the data of "Private Settings" section with anybody!!!

#define OPERATOR "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
#define OPERATOR "YRZVDJHUSNYBQFIWUWCVSEQVBLWBIMNOUUVJKWXLSAAODNHMZYQASNWDMQNA"
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert this

const unsigned short computorID = request->nonce % NUMBER_OF_COMPUTORS;

ACQUIRE(gCustomMiningSharesCountLock);
gCustomMiningSharesCount[computorID]++;
Copy link
Contributor

@krypdkat krypdkat Apr 12, 2025

Choose a reason for hiding this comment

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

If there is no verifier, the nodes consider all shares are valid by default.
So when verifier tells node that nonce_X isn't valid, we do gCustomMiningSharesCount[nonce_X % 676]--. Otherwise, no action needed.
(need to re-think)
It would be nice to have a flag indicating if nonce_X is verified or not

@cyber-pc cyber-pc changed the title Custom mining with verification. [INVALID] Custom mining with verification. Apr 14, 2025
@cyber-pc cyber-pc closed this Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants