-
Notifications
You must be signed in to change notification settings - Fork 31
feat(frontend): integrate pow into loader and guard logic #5890
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-7 # Conflicts: # src/frontend/src/lib/services/loader.services.ts # src/frontend/src/tests/lib/services/loader.services.spec.ts
…sistency. Adjusted function parameters formatting to improve readability.
…tect-allow-signing-7
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.
Before anything, may you please provide an explanation in the description on why it will be ok that the component AddressGuard is agnostic to being allowed to sign? Like we discussed offline.
Most important part is: we are removing the check for the signer-allowance-success. Why?
@@ -18,11 +19,13 @@ | |||
let signerAllowanceLoaded = false; | |||
const loadSignerAllowanceAndValidateAddresses = async () => { |
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 now can be renamed?
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 the pow environment naming has been fixed
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 mean, if you are going to remove the signer allowance, you can rename this function no?
My understanding is that once the feature is complete, the feature flag is gone, so that part inside the IF condition disappear. Or not?
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 recommend to leave the Feature flag, so that we can disable it quickly if there are issues with the PoW in production (e.g. to much CPU overhead, or not enough Cycles per time period are allowance (fine-tuning of settings)).
…ntend/pow/protect-allow-signing-7
…tect-allow-signing-7
Motivation
This Pull Request introduces a conditional feature toggle (POW_FEATURE_ENABLED) to support or bypass Proof-of-Work-related functionality. Modifications are made to reflect this toggle in address validation, component hierarchy, and loading logic.
Changes
Introduced POW_FEATURE_ENABLED usage to conditionally skip initSignerAllowance logic.
Updated component hierarchy to include a new wrapper component, PowProtector, for Proof-of-Work protection.
Incorporated POW_FEATURE_ENABLED to bypass conditional logic for invoking initSignerAllowance in the loading process.
Tests