Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const payout = await calc({ | ||
| userId: user.id, | ||
| projectType: cert.projectType, | ||
| verdict: verdict.toLowerCase() as 'approved' | 'rejected', | ||
| certCreatedAt: cert.createdAt, | ||
| customBounty: cert.customBounty, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Please provide a more detailed explanation on what was changed. The PR description is not enough. |
|
It seems that you've decreased hardware payout and increased apps. Who exactly was this approved by? Respectfully none of this has been run by us. |
|
|
| </BarChart> | ||
| )} | ||
| </ResponsiveContainer> | ||
| ) |
There was a problem hiding this comment.
Why is half your PR editing indentation?
There was a problem hiding this comment.
It's just pnpm pretty
@EricZil wanted this in the last 2 PRs :)
Nullskulls
left a comment
Nullskulls
left a comment
There was a problem hiding this comment.
I will not be moving forward with review of this PR seeing as how none of this was discussed with any of us.
This was disccused with Dhamari, hardware should be decreased as the reviewer doesn't need to install anything to thier system, so it's just reading the files/watching a demo For apps, you need an emulator to test it which isn't really easy to get :) Also this increase was offset by a decrease in web applications, which represent approximately 50%. |
Okay, this was set to motivate shipwrights to do at least 1 review a day, it's just a very small bonus to motivate inactive shipwrights to do one review and if they did one, it's very likely they will continue doing more certs :) |
|
|
||
| if (verdict.toLowerCase() === 'approved' || verdict.toLowerCase() === 'rejected') { | ||
| const payout = await calc(user.id, cert.projectType, cert.customBounty) | ||
| const payout = await calc({ | ||
| userId: user.id, | ||
| projectType: cert.projectType, | ||
| verdict: verdict.toLowerCase() as 'approved' | 'rejected', | ||
| certCreatedAt: cert.createdAt, | ||
| customBounty: cert.customBounty, |
There was a problem hiding this comment.
Bug: A race condition exists where concurrent review certifications can both receive the same daily bonus multiplier, as the payout is calculated before the review completion is saved to the database.
Severity: MEDIUM
Suggested Fix
Wrap the critical section of the PATCH endpoint, from reading the review count with getDailyMulti() to updating the shipCert record, within a prisma.$transaction(). This will ensure the read and write operations are atomic, preventing other requests from reading stale data.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: sw-dash/src/app/api/admin/ship_certifications/[id]/route.ts#L236-L243
Potential issue: The payout multiplier is calculated by `calc()` before the database
update commits the review's `reviewCompletedAt` timestamp. If two certification requests
are processed concurrently, both calls to `getDailyMulti()` could query the database
before either review is marked as complete. This would cause both requests to
incorrectly receive the same tiered bonus, such as the "first review of the day" 1.5x
multiplier, leading to users earning duplicate rewards. The issue is present for
multiple bonus thresholds.
2 participants
Changed the payouts system to a more engaging and rewarding ones where we reward hard & old projects more than the web apps/new stuff :)