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

Accept transfer #310

Merged
merged 6 commits into from
Dec 20, 2023
Merged

Accept transfer #310

merged 6 commits into from
Dec 20, 2023

Conversation

guibescos
Copy link
Contributor

@guibescos guibescos commented Dec 20, 2023

Refactor accept transfer
Add feature to be compatible with someone having multiple accounts

@guibescos guibescos requested a review from cctdaniel as a code owner December 20, 2023 17:16
Copy link

vercel bot commented Dec 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
staking ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 5:44pm
staking-devnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 5:44pm

@@ -18,14 +18,14 @@ const RequestSplit: NextPage = () => {
const { connection } = useConnection()
const anchorWallet = useAnchorWallet()

const [owner, setOwner] = useState<PublicKey>()
const [recipient, setRecipient] = useState<PublicKey>()
Copy link
Contributor Author

@guibescos guibescos Dec 20, 2023

Choose a reason for hiding this comment

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

Rename owner to recipient

break
}
}
}

useEffect(() => {
Copy link
Contributor

@0xfirefist 0xfirefist Dec 20, 2023

Choose a reason for hiding this comment

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

Maybe use useStakeConnection hook here and in request.tsx too?

@@ -88,7 +88,7 @@ const RequestSplit: NextPage = () => {
)
Copy link
Contributor

Choose a reason for hiding this comment

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

this comment refers to line 69
Can replace the useEffect hook with useStakeAccounts hook.


const ApproveSplit: NextPage = () => {
const { connection } = useConnection()
const anchorWallet = useAnchorWallet()
const { publicKey, connected } = useWallet()
const [amount, setAmount] = useState<PythBalance>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can combine amount and recipient in an object. Since we are setting them always together.

Having an object result in one state update. Else, it will be multiple state updates. Imo is a good practice to have as less state update as possible.

@guibescos
Copy link
Contributor Author

Sorry I really need this PR right now, let me fix comments later

@guibescos guibescos merged commit c600e61 into main Dec 20, 2023
8 checks passed
@guibescos guibescos deleted the request-transfer branch December 20, 2023 18:51
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.

2 participants