-
Notifications
You must be signed in to change notification settings - Fork 30
feat(frontend): implement pow worker and service layer #5887
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
base: main
Are you sure you want to change the base?
Conversation
…tect-allow-signing-6
… the allowSigning function.
…ation of the allowSigning function." This reverts commit 0f19642.
… the allowSigning function.
…nto feat/frontend/pow/protect-allow-signing-4
…signing-4' into feat/frontend/pow/protect-allow-signing-4
…tect-allow-signing-4
…signing-4' into feat/frontend/pow/protect-allow-signing-4
…d change signature to support no arguments
…tect-allow-signing-4 # Conflicts: # src/frontend/src/tests/lib/canisters/backend.canister.spec.ts
…d change signature to support no arguments
…chedule interval for the pow worker
…chedule interval for the pow worker
…tect-allow-signing-4
…signing-4' into feat/frontend/pow/protect-allow-signing-4
|
||
// Otherwise, increment the nonce (bigint increment) and try again | ||
nonce++; | ||
} |
There 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.
maybe make it functional? maybe with self-called functions?
const nonce = ...
There 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 can do that but It could reduce readability. Since this is a critical code a more 'procedural' presentation could therefore be desired here. I will give it a try.
There 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.
thank you! in any case, if you prefer this implementation, maybe preferable to use while (prefix > target)
and refactor the logic?
There 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.
A do-while-loop and a conditional-loop will be less readable since they duplicate the condition.
There 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.
Yes, my idea was something like
const target = BigInt(Math.floor(0xffffffff / difficulty));
const solvePowChallengeRecursive = async (
timestamp: bigint,
nonce: bigint = 0n
): Promise<bigint> => {
const challengeStr = `${timestamp}.${nonce}`;
const hashHex = await hashText(challengeStr);
const prefix = BigInt(parseInt(hashHex.slice(0, 8), 16));
return prefix <= target
? nonce
: solveChallengeRecursive(timestamp, nonce + 1n);
}
const nonce = await solvePowChallengeRecursive(...)
But anyway, let's proceed with the while
loop for readability. May you please change it to have a condition instead of while(true)
There 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.
Ah! and for safety, let's put an hard coded limit of N iterations that raise an error
P.S. and tests
difficulty | ||
}: { | ||
timestamp: bigint; // The unique timestamp for the challenge | ||
difficulty: number; // The difficulty level |
There 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.
let's use the docstring like the other functions, so that you define the params and the results directly in it
There 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 agree. I will add a complete docstring here.
There 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.
remove them from here?
Yes let's add a line break here Co-authored-by: Antonio Ventilii <[email protected]>
LGTM Co-authored-by: Antonio Ventilii <[email protected]>
There 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.
Added feedback on comments
difficulty | ||
}: { | ||
timestamp: bigint; // The unique timestamp for the challenge | ||
difficulty: number; // The difficulty level |
There 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 agree. I will add a complete docstring here.
|
||
// Otherwise, increment the nonce (bigint increment) and try again | ||
nonce++; | ||
} |
There 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 can do that but It could reduce readability. Since this is a critical code a more 'procedural' presentation could therefore be desired here. I will give it a try.
…tect-allow-signing-4
…signing-4' into feat/frontend/pow/protect-allow-signing-4
…tect-allow-signing-4
|
||
// Otherwise, increment the nonce (bigint increment) and try again | ||
nonce++; | ||
} |
There 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.
Ah! and for safety, let's put an hard coded limit of N iterations that raise an error
P.S. and tests
Motivation
This change introduces the foundational components required to support a Proof-of-Work (PoW)–protected
allowSigning
operation. Specifically, it prepares a scheduler that will execute therequestSignerCycles
function within a Web Worker context.Note: The PoW worker is defined but not yet initialized via
initPowProtectorWorker(..)
. This will be completed in a subsequent pull request.Changes
Scheduler
-compliant scheduler using the existingSchedulerTimer
to support timed execution of PoW jobs.PowProtectorWorker
via theinitPowProtectorWorker
function.PowProtectorWorker
).PowProtectorWorkerInitResult
to describe the worker's initialization return structure, and introduced the newPowProtectorWorker
type.onPowProtectionMessage
for processingpostMessage
communications from the Web Worker on the application side.onPowProtectionMessage(msg)
to the the globalonmessage
handler within the web application to handle messages dispatched by the PoW worker.