-
Notifications
You must be signed in to change notification settings - Fork 15
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
Check if stake addresses in proposals are registered onchain #963
base: master
Are you sure you want to change the base?
Conversation
159e02a
to
0a060c4
Compare
7b9d4ca
to
b81a3ea
Compare
[ stakeCred | ||
| (proposal, _) <- proposals | ||
, let (_, _, govAction) = fromProposalProcedure eon proposal | ||
, TreasuryWithdrawal withdrawalsList _ <- [govAction] -- Match on TreasuryWithdrawal action |
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 I said during the pair programming session, I don't like pattern matching in list comprehensions generally speaking. But in this case, it would be way more verbose to do differently, so I'm fine with this version.
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 was going to say I thought it was an interesting way of pattern-matching
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 haven't tested it, but code looks good to me 👍
I just wrote a suggestion.
let returnAddrHashes = | ||
Set.fromList | ||
[ StakeCredentialByKey returnAddrHash | ||
| (proposal, _) <- proposals | ||
, let (_, returnAddrHash, _) = fromProposalProcedure eon proposal -- fromProposalProcedure needs to be adjusted so that it works with script hashes. | ||
] | ||
treasuryWithdrawalAddresses = | ||
Set.fromList | ||
[ stakeCred | ||
| (proposal, _) <- proposals | ||
, let (_, _, govAction) = fromProposalProcedure eon proposal | ||
, TreasuryWithdrawal withdrawalsList _ <- [govAction] -- Match on TreasuryWithdrawal action | ||
, (_, stakeCred, _) <- withdrawalsList -- Extract fund-receiving stake credentials | ||
] | ||
allAddrHashes = Set.union returnAddrHashes treasuryWithdrawalAddresses |
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 it would be nicer to pattern-match only once on each proposal. I would make a getStakeAddressesFromProposal
function that returns a Set
and just mconcatMap
over it. Maybe a getStakeAddressFromGovAction
inside. But this is fine, just an idea, up to you.
Changelog
Context
Resolves #900
How to trust this PR
Tested locally with a Treasury Withdrawal action where neither the deposit return or funds receiving stake addresses are registered:
Checklist