-
Notifications
You must be signed in to change notification settings - Fork 1
Add prize API #589
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
Add prize API #589
Conversation
raymondjacobson
left a comment
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.
flow looks super good. i think table design could probably be a bit simpler
api/v1_wheel_spin.go
Outdated
| "go.uber.org/zap" | ||
| ) | ||
|
|
||
| type WheelSpinRequest struct { |
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.
could we maybe generalize this to just "prizes" instead of wheel spin?
tables being
prizes
claimed_prizes
| @@ -0,0 +1,22 @@ | |||
| BEGIN; | |||
|
|
|||
| CREATE TABLE IF NOT EXISTS wheel_spin_configs ( | |||
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 think this table isn't super necessary. The prizes themselves could refer directly to the mint and I think we'd save ourselves some work / complexity
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.
makes sense, the idea here was if we have new wheels for same mint but different spin amounts. but you think it's fine to not build that in for now?
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.
yeah i don't think we need to have it but i see your point. i'm okay doing it but maybe instead of wheels we could call it a prize group or something
| } | ||
| defer prizeRows.Close() | ||
|
|
||
| var prizes []Prize |
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.
nice
api/v1_wheel_spin.go
Outdated
| FROM wheel_spin_prizes | ||
| WHERE config_id = $1 AND is_active = true | ||
| ORDER BY id | ||
| `, configID) |
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 think the config isn't a necessary step really if we put amount and mint into the prizes table, which I think would be a bit simpler
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.
yeah down to go with this. a thought though: to determine which prizes are active in the wheel and what there percentages are, should that actually be defined in the config table or also just do in the prizes table?
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 think it doesn't need to be that generalized yet but fair enough. i'm okay either way
api/v1_prizes_claim.go
Outdated
| } | ||
|
|
||
| // Get active prizes for this config | ||
| // Use writePool to ensure we see the latest data (important in tests, and safe in production) |
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.
why is the write pool needed? the replication is super fast and idt we need it for the tests?
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.
agreed not needed
api/v1_wheel_spin.go
Outdated
| &balanceChange.Account, | ||
| ) | ||
| } else { | ||
| // If mint not provided, find any spend transaction for this wallet |
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 think prob best to always assert having a signature
raymondjacobson
left a comment
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.
lgtm can go either way on the tables but i think we should stay away from "wheel" as the name for these prize groups or prize pools
| yakMintAddress = "ZDaUDL4XFdEct7UgeztrFQAptsvh4ZdhyZDZ1RpxYAK" | ||
| yakClaimAmount = 2000000000 // 2 YAK with 9 decimals - amount required to claim a prize | ||
| yakAirdropAmount = 1000000000 // 1 YAK with 9 decimals - amount awarded in coin airdrop prizes | ||
| prizeReceiverAddress = "EHd892m3xNWGBuAXnafavqcFjXXUZp9bGecdSDNP2SLR" |
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.
what address is this?
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.
This is just a random wallet I own for testing purposes
Adds /prizes endpoint that takes wallet, and signature, verifies user spent the correct amount of coin and sent to the right address, and then randomly gives user a claimed_prize