-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
feat: algorand grants integration #9168
Conversation
4dc589d
to
31e93d3
Compare
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! just few comments
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.
great job man!
|
||
if ( | ||
not eth_payout_address and not zcash_payout_address and | ||
not celo_payout_address and not zil_payout_address and | ||
not polkadot_payout_address and not kusama_payout_address and | ||
not harmony_payout_address and not binance_payout_address and | ||
not rsk_payout_address | ||
not rsk_payout_address and not algorand_payout_address |
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.
It looks like we have a lot of places here where we have to check for every chain in these conditionals. Rather than repeating these over and over, could we make a set and then check the set? For example with these payout addresses, rather than doing this long list of not
s would it be cleaner to add these to a set and then check if the set is empty?
all_payout_address_types = {eth_payout_address, zcash_payout_address, celo_payout_address, zil_payout_address, polkadot_payout_address, kusama_payout_address, harmony_payout_address, binance_payout_address, rsk_payout_address, algorand_payout_address}
if not all_payout_address_types:
response['message'] = 'error: payout_address is a mandatory parameter'
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.
Does that work @zacheryschiller ?
I'd say we'd do it in a followup PR
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.
as synced up offline -> will do this in a follow up PR
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.
My bad. Should be if not any(all_payout_address_types):
Description
This PR aims at integrating algorand into grants.
It uses the algosigner to contribute to grants
Steps tested
Grant Creation
https://share.vidyard.com/watch/4z4rhUbCNw3jhAttzy47LK?
Grant Payout
https://share.vidyard.com/watch/oqoMReMN4f6M9UTQuCNaGG?
Backend Sync
Fixes GITC-65