Erc721 vote button #2680

adamgall · 2025-01-21T17:38:42Z

Closes https://linear.app/decent-labs/issue/ENG-136/contango-couldnt-vote

Currently, users are unable to vote on ERC 721 Token proposals. The "Vote" button does not appear when loading the app with a wallet connected that DOES have voting power on a proposal that already exists is and open for voting.

This PR addresses that bug.

Fixed two bugs, and do some cleanup.

Bugs fixed:

"zero is not undefined". When the tokenId of an owned NFT by the connected address is 0, that was returning truthy in a place where we actually want to check if it's undefined.
Refactor VoteContext to improve voting eligibility checks and update user reference handling
a. Modified getCanVote function to accept remainingTokenIdsLength as a parameter for better clarity and efficiency.
b. Updated user reference management to use a hex representation of the user's address combined with the length of remaining token IDs.
c. Ensured that the voting eligibility logic remains intact while enhancing readability and maintainability.

Cleanup:

Don't need to check a boolean twice in order to render one thing
Remove unused functions from VoteContext value

Testing:

Use Rabby
Add Josh's address as a "read only" address in Rabby: 0xda38c543596b7E1b79ad0aC2C7797ADb914Ea3C5
Load up the VANTA DAO: eth:0xB14AC30AA97057e2A934c47BDc45171335B91Bd8
Connect to the Decent app with Rabby using Josh's address
Load up a current active proposal.
a. Before this PR: no Vote button
b. In this PR: Vote button

adamgall · 2025-01-21T17:40:20Z

src/components/Proposals/ProposalActions/CastVote.tsx

-  const { canVote, canVoteLoading, hasVoted, hasVotedLoading } = useVoteContext();
-
-  // if the user is not a signer or has no delegated tokens, don't show anything
-  if (!canVote) {
-    return null;
-  }


adamgall · 2025-01-21T17:40:55Z

src/components/Proposals/ProposalVotes/context/VoteContext.tsx

-  getCanVote: () => Promise<void>;
-  getHasVoted: () => void;
 }

 const VoteContext = createContext<IVoteContext>({
  canVote: false,
  canVoteLoading: false,
  hasVoted: false,
  hasVotedLoading: false,
-  getCanVote: async () => {},
-  getHasVoted: () => {},


adamgall · 2025-01-21T17:43:22Z

src/components/Proposals/ProposalVotes/context/VoteContext.tsx

+  const getCanVote = useCallback(
+    async (remainingTokenIdsLength: number) => {
+      setCanVoteLoading(true);
+      let newCanVote = false;
+      if (userAccount.address && publicClient) {
+        if (snapshotProposal) {
+          const votingWeightData = await loadVotingWeight();
+          newCanVote = votingWeightData.votingWeight >= 1;
+        } else if (governance.type === GovernanceType.AZORIUS_ERC20) {
+          const azoriusProposal = proposal as AzoriusProposal;
+          const ozLinearVotingContract = getContract({
+            abi: abis.LinearERC20Voting,
+            address: azoriusProposal.votingStrategy,
+            client: publicClient,
+          });
+          newCanVote =
+            (await ozLinearVotingContract.read.getVotingWeight([
+              userAccount.address,
+              Number(proposal.proposalId),
+            ])) > 0n;
+        } else if (governance.type === GovernanceType.AZORIUS_ERC721) {
+          const votingWeight = await erc721VotingWeight();
+          newCanVote = votingWeight > 0n && remainingTokenIdsLength > 0;
+        } else if (governance.type === GovernanceType.MULTISIG) {
+          newCanVote = !!safe?.owners.includes(userAccount.address);
+        } else {
+          newCanVote = false;
+        }
      }
-    }

-    if (canVote !== newCanVote) {
-      setCanVote(newCanVote);
-    }
-    setCanVoteLoading(false);
-  }, [
-    userAccount.address,
-    publicClient,
-    canVote,
-    snapshotProposal,
-    governance.type,
-    loadVotingWeight,
-    remainingTokenIds.length,
-    safe?.owners,
-    proposal,
-    erc721VotingWeight,
-  ]);
+      if (canVote !== newCanVote) {
+        setCanVote(newCanVote);
+      }
+      setCanVoteLoading(false);
+    },
+    [
+      userAccount.address,
+      publicClient,
+      canVote,
+      snapshotProposal,
+      governance.type,
+      loadVotingWeight,
+      safe?.owners,
+      proposal,
+      erc721VotingWeight,
+    ],
+  );

-  const connectedUserRef = useRef<Address>();
+  const connectedUserRef = useRef<Hex>();
  useEffect(() => {
-    const isUserRefCurrent = connectedUserRef.current === userAccount.address;
+    const refValue = toHex(`${userAccount.address}-${remainingTokenIds.length}`);
+    const isUserRefCurrent = connectedUserRef.current === refValue;
    if (!isUserRefCurrent) {
-      connectedUserRef.current = userAccount.address;
-      getCanVote();
+      connectedUserRef.current = refValue;
+      getCanVote(remainingTokenIds.length);
    }
-  }, [getCanVote, userAccount.address]);
+  }, [getCanVote, userAccount.address, remainingTokenIds.length]);


adamgall · 2025-01-21T17:43:38Z

src/components/Proposals/ProposalVotes/context/VoteContext.tsx

-      getCanVote,
-      getHasVoted,
    };
-  }, [canVote, canVoteLoading, getCanVote, getHasVoted, hasVoted, hasVotedLoading]);


adamgall · 2025-01-21T17:43:48Z

src/hooks/DAO/proposal/useUserERC721VotingTokens.ts

+              if (!to || !from || tokenId === undefined) {
+                throw new Error('An ERC721 event has undefined data');


adamgall · 2025-01-21T17:46:13Z

src/components/Proposals/ProposalVotes/context/VoteContext.tsx

+  const connectedUserRef = useRef<Hex>();
  useEffect(() => {
-    const isUserRefCurrent = connectedUserRef.current === userAccount.address;
+    const refValue = toHex(`${userAccount.address}-${remainingTokenIds.length}`);
+    const isUserRefCurrent = connectedUserRef.current === refValue;
    if (!isUserRefCurrent) {
-      connectedUserRef.current = userAccount.address;
-      getCanVote();
+      connectedUserRef.current = refValue;
+      getCanVote(remainingTokenIds.length);
    }
-  }, [getCanVote, userAccount.address]);
+  }, [getCanVote, userAccount.address, remainingTokenIds.length]);


This is the important part here. We need to make sure getCanVote is ran every time the remainingTokenIds.length changes, which happens asynchronously and definitely is updated after the user's address ref was first being set.

See also

https://github.com/decentdao/decent-interface/pull/2680/files#diff-ef559e0f3d0f16b9b9ff38a3044b1ec3581ce650bd358f2e16a7a6939476396bR119-R120

DarksightKellar · 2025-01-21T18:06:37Z

Does this need to hotfix to prod?

adamgall · 2025-01-21T18:22:18Z

Does this need to hotfix to prod?

Oh yeah good point. Let me confirm that it's safe to rebase and make it happen

…e tree

…user reference handling - Modified `getCanVote` function to accept `remainingTokenIdsLength` as a parameter for better clarity and efficiency. - Updated user reference management to use a hex representation of the user's address combined with the length of remaining token IDs. - Ensured that the voting eligibility logic remains intact while enhancing readability and maintainability.

cloudflare-workers-and-pages · 2025-01-21T18:27:32Z

Deploying decent-interface with Cloudflare Pages

Latest commit:	`fe943f0`
Status:	✅ Deploy successful!
Preview URL:	https://f8d33b5c.decent-interface.pages.dev
Branch Preview URL:	https://erc721-vote-button.decent-interface.pages.dev

View logs

adamgall · 2025-01-21T18:41:49Z

@Da-Colon @DarksightKellar just to confirm, have you both successfully reproduced the bug and then confirmed that this PR fixes it?

Just considering this is a hotfix into prod, I want to be sure every "approval" includes QA testing.

See the "Testing" section in the top comment.

johnqh · 2025-01-21T19:49:35Z

Created ticket

johnqh

Tested complete flow with deployment, created proposal and voted successfully.

linear · 2025-01-21T19:52:47Z

ENG-136 Contango couldn't vote

adamgall · 2025-01-21T19:55:24Z

@johnqh specifically, did you load the app fresh, with the proposal already created? Like, if you just refresh the page/app, does the proposal still give you the Vote button?

mudrila

Tested on Contango /proposals/5?dao=eth:0xB14AC30AA97057e2A934c47BDc45171335B91Bd8 with the wallet that has NFT for voting - works good.
One caveat, but I think I've seen this some time ago - see screenshot. Right after voting the voting option appears as empty box but after refresh it loads correctly. I'll add a ticket for that in Linear

adamgall requested review from johnqh, DarksightKellar, mudrila and Da-Colon January 21, 2025 17:38

adamgall self-assigned this Jan 21, 2025

adamgall commented Jan 21, 2025

View reviewed changes

Da-Colon approved these changes Jan 21, 2025

View reviewed changes

DarksightKellar approved these changes Jan 21, 2025

View reviewed changes

adamgall added 4 commits January 21, 2025 13:27

Zero is not undefined

5fae579

We're already checking this before even including the component in th…

4ce2d66

…e tree

Remove unused functions from Vote Context value

a41fb5f

adamgall force-pushed the erc721-vote-button branch from 5af9c96 to fe943f0 Compare January 21, 2025 18:27

adamgall changed the base branch from develop to hotfix/v0.4.4 January 21, 2025 18:29

johnqh approved these changes Jan 21, 2025

View reviewed changes

mudrila approved these changes Jan 21, 2025

View reviewed changes

adamgall mentioned this pull request Jan 21, 2025

Hotfix/v0.4.4 #2682

Merged

adamgall merged commit 0e9dcd7 into hotfix/v0.4.4 Jan 21, 2025
4 checks passed

adamgall deleted the erc721-vote-button branch January 21, 2025 20:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Erc721 vote button #2680

Erc721 vote button #2680

adamgall commented Jan 21, 2025 •

edited

Loading

adamgall Jan 21, 2025

adamgall Jan 21, 2025

adamgall Jan 21, 2025

adamgall Jan 21, 2025

adamgall Jan 21, 2025

adamgall Jan 21, 2025

adamgall Jan 21, 2025 •

edited

Loading

DarksightKellar commented Jan 21, 2025

adamgall commented Jan 21, 2025

cloudflare-workers-and-pages bot commented Jan 21, 2025 •

edited

Loading

adamgall commented Jan 21, 2025 •

edited

Loading

johnqh commented Jan 21, 2025

johnqh left a comment

linear bot commented Jan 21, 2025

adamgall commented Jan 21, 2025

mudrila left a comment •

edited

Loading

		if (!to \|\| !from \|\| tokenId === undefined) {
		throw new Error('An ERC721 event has undefined data');

Erc721 vote button #2680

Erc721 vote button #2680

Conversation

adamgall commented Jan 21, 2025 • edited Loading

adamgall Jan 21, 2025

Choose a reason for hiding this comment

adamgall Jan 21, 2025

Choose a reason for hiding this comment

adamgall Jan 21, 2025

Choose a reason for hiding this comment

adamgall Jan 21, 2025

Choose a reason for hiding this comment

adamgall Jan 21, 2025

Choose a reason for hiding this comment

adamgall Jan 21, 2025

Choose a reason for hiding this comment

adamgall Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

DarksightKellar commented Jan 21, 2025

adamgall commented Jan 21, 2025

cloudflare-workers-and-pages bot commented Jan 21, 2025 • edited Loading

Deploying decent-interface with Cloudflare Pages

adamgall commented Jan 21, 2025 • edited Loading

johnqh commented Jan 21, 2025

johnqh left a comment

Choose a reason for hiding this comment

linear bot commented Jan 21, 2025

adamgall commented Jan 21, 2025

mudrila left a comment • edited Loading

Choose a reason for hiding this comment

adamgall commented Jan 21, 2025 •

edited

Loading

adamgall Jan 21, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 21, 2025 •

edited

Loading

adamgall commented Jan 21, 2025 •

edited

Loading

mudrila left a comment •

edited

Loading