Skip to content
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

Creating staked expenditure #994

Merged

Conversation

jakubcolony
Copy link
Collaborator

@jakubcolony jakubcolony commented Aug 23, 2023

Description

This PR adds the functionality of creating a staked expenditure.

For context, you can only create an expenditure if you have the Administration permissions in the colony. Staked Expenditure extension allows anyone to create an expenditure, as long as they stake it. The required stake amount is a fraction of the total domain reputation that an expenditure is created in, which is configured when enabling the extension.

Testing

Creating a regular (non-staked) expenditure

Before installing any extensions, try creating a new expenditure. It should work exactly the same as previously.

Creating a staked expenditure

  1. Enable reputation auto mining: http://localhost:3001/reputation/monitor/toggle
  2. Mint some tokens, pay yourself and wait for the mining cycle to finish (you can do step 3 while you're waiting).
  3. Install and enable Staked Expenditure extension. Enter the desired stake fraction.
  4. Activate all your tokens.
  5. Create an expenditure. You should see a dialog appear with the required stake amount. Since at this point you're the only user with reputation, the amount should be roughly equal to the fraction percentage times the amount of tokens you've paid yourself:
image
  1. The saga with 3 transactions (approve stake, make expenditure, and set expenditure values) should go through and you should be redirected to the expenditure page.
  2. Check the token popover to confirm the stake amount is shown there.

Not enough active tokens

Deactivate all your tokens and try to create an expenditure. The stake dialog should inform you you don't have enough active tokens:
image

Editing, locking, funding & finalizing

Should work exactly as previously.

Claiming a payout

The function determining which payouts are claimable now uses the block time instead of machine time. If you disable auto mining, you should see the correct time until next claim (relative to the machine time):
image

Note this will only be correct straight after reload, as the machine time advances while the dev blockchain time doesn't. In the example above you can advance the time by 60 seconds using the approach from #925.

Creating an expenditure as a non-admin user

Try paying some tokens to another dev wallet/metamask, then follow the steps above to confirm you can create a staked expenditure even if you don't have the Administration permissions. You should also be able to edit, lock and finalize, but not fund (since you're missing permissions to move funds).

Resolves #952

@jakubcolony jakubcolony self-assigned this Aug 23, 2023
@jakubcolony jakubcolony linked an issue Aug 24, 2023 that may be closed by this pull request
@@ -25,7 +28,7 @@ const useStakingControls = (limitExceeded: boolean) => {

const {
userActivatedTokens,
enoughTokensToStakeMinimum,
hasEnoughTokens: enoughTokensToStakeMinimum,
loadingUserTokenBalance,
} = useEnoughTokensForStaking(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willm30 I nicked your hook and made it more generic to use for staking expenditures too.

Comment on lines +46 to +47
{({ watch }) => {
const formValues = watch();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Dialogs can't access the form context since they're rendered in a different place in the DOM.

import styles from './StakeExpenditureDialog.module.css';

const useRequiredStakeAmount = (colony: Colony, selectedDomainId: number) => {
const { data } = useGetUserReputationQuery({
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to get rid of the getUserReputation lambda altogether. All it does it uses colonyJS to call the reputation oracle endpoint, without any on-chain interaction. It's probably faster just to call that endpoint directly from the front end instead of going through a GQL query.

Comment on lines -35 to -44
if (extensionId === Extension.VotingReputation) {
return initializationParams?.reduce((formattedPayload, { paramName }) => {
if (paramName.endsWith('Period')) {
return {
...formattedPayload,
[paramName]: new Decimal(payload[paramName])
.mul(3600)
.toFixed(0, Decimal.ROUND_HALF_UP),
};
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it non-obvious so I moved the control over transforming the params to the params configuration in ~constants/extensions. Now this function simply applies pre-defined transformations.

Comment on lines +15 to +24
const fetchCurrentBlockTime = async () => {
const { ethersProvider } = wallet;

const block = await ethersProvider?.getBlock('latest');
if (!block) {
return;
}

setCurrentBlockTime(block.timestamp * 1000);
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's worth having a lambda to do just this but would be good to know your thoughts @rdig

Comment on lines +68 to +72
const [permissionDomainId, childSkillIndex] = yield getPermissionProofs(
colonyClient,
createdInDomain.nativeId,
ColonyRole.Administration,
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty much all that's needed to replace the withProofs functions. And in case of motions it will be quicker, since we have the motion domain cached, whereas colonyJS makes an additional contract call to get it.

@@ -85,6 +85,7 @@ export const getBasicWallet = async (lastWallet: LastWallet) => {
address: lastWallet.address,
label: lastWallet.type,
chains: [{ id: getChainIdAsHex(network.chainId), namespace: 'evm' }],
ethersProvider: provider,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this property to the wallet object to be able to call getBlock in useCurrentBlockTime.

@jakubcolony jakubcolony marked this pull request as ready for review August 24, 2023 13:13
@jakubcolony jakubcolony requested review from rdig and willm30 August 24, 2023 13:13
}

/**
* @TODO: This should include a permissions check as users with
Copy link
Contributor

@willm30 willm30 Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getUserRolesFromDomain should serve you here (in src/transformers/index.ts)

Copy link
Contributor

@willm30 willm30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, aside from a couple of observations:

  1. Claiming the funds is not removing them from "staked" in the token activation popover

  2. I believe the wrong slots are being shown in expenditures.

E.g. Expenditure 3 UI:

expenditure3

Versus Expenditure 3 DB:

expend3

The recipient address and amount are actually from expenditure 2:

expend2

Possibly caused by slots not having unique ids and apollo mixing them up? That would be my first guess.

There's definitely something funky going on since when I click back on expenditure 3, it has the correct slot:

slot3

Let me know if you want to address these elsewhere and I can approve this PR, or if you'll address them here.

@jakubcolony
Copy link
Collaborator Author

@willm30

  1. Claiming the funds is not removing them from "staked" in the token activation popover
    There's a separate issue for that: Reclaiming staked expenditure stake #1006

I couldn't reproduce the wrong slots issue myself but indeed all the slots were being cached under a single cache entry. I disabled caching of that type entirely which should hopefully fix the problem.

@jakubcolony jakubcolony requested a review from willm30 August 29, 2023 22:21
@jakubcolony jakubcolony merged commit 494a045 into feat/advanced-payments Aug 30, 2023
@jakubcolony jakubcolony deleted the feat/925-creating-staked-expenditure branch August 30, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating staked expenditures
2 participants