Skip to content

added modal for staking #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

Merged
merged 15 commits into from
May 17, 2025
Merged

added modal for staking #15

merged 15 commits into from
May 17, 2025

Conversation

sul-igr
Copy link
Contributor

@sul-igr sul-igr commented Feb 23, 2025

Copy link

@sul-igr sul-igr marked this pull request as draft February 24, 2025 00:47
Copy link
Contributor

@microHoffman microHoffman left a comment

Choose a reason for hiding this comment

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

mostly good, just few comments and also feature changes here and in notion

pages/index.vue Outdated
@@ -296,6 +308,12 @@ const chainIdToToggleTo = computed(() => getChainIdToToggleTo(chainId.value))
const switchChain = () => {
toggleActiveChain(chainId.value)
}

const createStakeModal = ref()
Copy link
Contributor

Choose a reason for hiding this comment

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

question: hmm that's an interesting approach but i'm wondering what is a benefit of doing this instead of just simple store that would hold the global values that we need to also access/set outside of the component itself?

Copy link
Member

@Skanislav Skanislav left a comment

Choose a reason for hiding this comment

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

tACK

if (pwnTokenBalance.value === undefined || pwnTokenBalance.value === 0n) {
return "0";
}
return formatDecimalPoint(formatUnits(pwnTokenBalance.value, 18));
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if decimals amount would even change but would make sense to reference it as a contsant

Copy link
Contributor

Choose a reason for hiding this comment

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

agree, updated

Comment on lines 342 to 351
// Add computed properties for potential voting power
// const potentialMultiplier = computed(() => {
// if (!lockUpEpochs.value) return 0
// return getMultiplierForLockUpEpochs(lockUpEpochs.value)
// })

// const potentialVotingPower = computed(() => {
// if (!stakeAmount.value || !lockUpEpochs.value) return '0'
// return getFormattedVotingPower(stakeAmount.value, potentialMultiplier.value)
// })
Copy link
Member

Choose a reason for hiding this comment

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

anything to do here?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, this was just unused after i've enabled the linter, removed the uncommented code

Comment on lines 80 to 99
console.log("Starting to send a transaction with following parameters:");
console.log(transaction);
// console.log(`Additional UI parameter passed to sendTransaction: step=${step?.text}, safeAddress=${safeAddress}`)

const { address: userAddress } = getAccount(wagmiAdapter.wagmiConfig)
const connectedChainId = getAccount(wagmiAdapter.wagmiConfig).chainId;
console.log(
`connectedChainId=${connectedChainId}; transaction.chainId=${transaction.chainId}`,
);

// @ts-expect-error probably some type mismatch between writeContract and simulateContract
await simulateContract(wagmiAdapter.wagmiConfig, transaction)
if (connectedChainId !== transaction.chainId) {
console.log(
`Switching chain from ${connectedChainId} to ${transaction.chainId}.`,
);
const switchedChain = await switchChain(wagmiAdapter.wagmiConfig, {
chainId: transaction.chainId!,
});
if (switchedChain.id !== transaction.chainId) {
throw new Error("User denied switching chains before sending a tx.");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Do you plan to keep these console.logs here?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes i think it does not hurt anything:)

@microHoffman microHoffman merged commit 389b874 into master May 17, 2025
1 check passed
@microHoffman microHoffman deleted the add-staking-action branch May 17, 2025 12:27
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.

3 participants