BAT gating for Web3 Brave Talk #1079
Conversation
Dsuh 943 bat gating
* Use renovate for dependency updates (#1054) 1. Disable dependabot for normal dependency updates. Note the dependabot PRs will need to be manually closed, and the "security" dependabot PRs will continue. 2. Use renovateapp instead. The renovate configuration included here is based on that from ads-serve, and: a. waits for updates to have been published for 4 days before raising a PR - this both reduces churn and also avoids the 3 day period where npm-published entries can be "unpublished" b. combines all minor and patch updates into a single PR * removed as *[bot] is not accepted by AWS (#1055) Signed-off-by: Ahmed Kamal <email.ahmedkamal@googlemail.com> * Update all non-major dependencies --------- Signed-off-by: Ahmed Kamal <email.ahmedkamal@googlemail.com> Co-authored-by: Graham Tackley <gtackley@brave.com> Co-authored-by: Ahmed Kamal <email.ahmedkamal@googlemail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* Use renovate for dependency updates (#1054) 1. Disable dependabot for normal dependency updates. Note the dependabot PRs will need to be manually closed, and the "security" dependabot PRs will continue. 2. Use renovateapp instead. The renovate configuration included here is based on that from ads-serve, and: a. waits for updates to have been published for 4 days before raising a PR - this both reduces churn and also avoids the 3 day period where npm-published entries can be "unpublished" b. combines all minor and patch updates into a single PR * removed as *[bot] is not accepted by AWS (#1055) Signed-off-by: Ahmed Kamal <email.ahmedkamal@googlemail.com> * Update all non-major dependencies (#1057) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Pin dependencies (#1056) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update all non-major dependencies (#1063) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update Node.js to v18.16.1 --------- Signed-off-by: Ahmed Kamal <email.ahmedkamal@googlemail.com> Co-authored-by: Graham Tackley <gtackley@brave.com> Co-authored-by: Ahmed Kamal <email.ahmedkamal@googlemail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Seems fine.
Part of security review https://github.com/brave/reviews/issues/1328
| header={"Non Zero BAT gating will be applied."} | ||
| subhead={ | ||
| "Only participants with non-zero BAT balance can join the call" | ||
| } |
There was a problem hiding this comment.
You should probably make these translatable text using useTranslation, see examples such as
There was a problem hiding this comment.
addressed in newest commit 7512714
src/components/web3/api.ts
Outdated
| network: string; // Use enum instead? | ||
| token: string; //optional | ||
| minimum: string; // in wei, e.g. 10e-18 |
There was a problem hiding this comment.
Comments are great, but even better if the code doesn't require a comment.
so something like:
| network: string; // Use enum instead? | |
| token: string; //optional | |
| minimum: string; // in wei, e.g. 10e-18 | |
| network: "ETH"; | |
| token?: string; | |
| minimum: string; // in wei, e.g. 10e-18 |
(In the example below network is always ETH, but if you supported others then I'd declare this type as e.g. network: "ETH" | "BAT". In typescript this is as good as where you'd need to use an enum in other languages.)
I'd usually suggest putting the units in the variable name where it's unclear what the unit value is (so minimumWEI rather than just minimum) but it looks like in this case this is an external API so you can't change the name.
There was a problem hiding this comment.
addressed in newest commit 7512714
src/hooks/use-web3-call-state.ts
Outdated
| @@ -48,6 +48,10 @@ export function useWeb3CallState( | |||
| const [moderatorNFTCollections, setModeratorNFTCollections] = useState< | |||
| NFTcollection[] | |||
| >([]); | |||
| const [minimumParticipantBATBalance, setMinimumParticipantBATBalance] = | |||
| useState<string>("1"); | |||
There was a problem hiding this comment.
Above you say minimum is in WEI, but here you set it to 1 which is either a huge amount or a tiny amount if I've understood what WEI means correctly?
There was a problem hiding this comment.
Where do setModeratorNFTCollections and setMinimumParticipantBATBalance get called from? If they're only needed for potential future expansion, you do not need to store them in state: hard code the hardcoded values in the json below.
Ideally code should reflect what it is doing now, not what it might do in some point in the future.
There was a problem hiding this comment.
addressed in newest commit 7512714. The unit is wei, thus 1 is very small (1e-18) but is sufficient for now since we are just doing non-zero BAT balance at the moment.
…ranslation rather than string literals
| header, | ||
| subhead, | ||
| loading = false, | ||
| children, |
There was a problem hiding this comment.
children looks like it's not used - if that's what was intended we should remove it.
This the PR that (pending sec review) will add the client side code for #943 (BAT gating) for Web3 Brave Talk. This will allow users to select a third (after NFTs and POAPs) option for gating their Web3 Calls in Brave Talk, which will only allow users with non zero BAT balance to join the call. Works as expected on desktop (dev2).