Feature: Implement wallet connection for Stellar #77

sotoJ24 · 2025-01-27T04:08:25Z

7️⃣ Create a Pull Request for issue #52 (PR)

! Changes

Add Stellar Testnet and Stellar Mainnet to configs/networks.json with the new "chain" field
Update pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx
Create separate reusable functions/components for starknetWallet and stellarWallet

Thank you for review this PR ✨

close: #52

Merge last changes

Marchand-Nicolas

It doesn't satisfy what was asked in the issue.

Marchand-Nicolas · 2025-01-27T08:20:21Z

.env.local

@@ -0,0 +1,20 @@
+NEXT_PUBLIC_DISCORD_CLIENT_ID=xxx


Don't commit your env file

Marchand-Nicolas · 2025-01-27T08:22:22Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

-
+import { connect as starknetConnect, disconnect as starknetDisconnect } from "starknetkit";
+// Import Stellar SDK (you'll need to add this dependency)
+import StellarSdk from "stellar-sdk";


You import libraries but do not install them...
Or you forgot to push your package.json

And you don't even use it

Marchand-Nicolas · 2025-01-27T08:25:14Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+
+async function stellarConnect() {
+  console.log("Connecting to Stellar wallet...");
+  const wallet = { address: "stellar-wallet-address" }; // Mock example


Do not mock it, the aim of this issue is to actually implement a stellar wallet

sotoJ24 · 2025-01-30T16:26:28Z

Dear @Marchand-Nicolas I have already made the commit for the changes

Marchand-Nicolas

Some code to clean. I will test the feature tomorrow

Marchand-Nicolas · 2025-01-30T17:11:56Z

configs/networks.json

@@ -2,11 +2,21 @@
  {
    "name": "mainnet",
    "url": "starknetid-mainnet.starknet.a5a.ch",
-    "indexer": true


Please don’t delete the indexer field

Marchand-Nicolas · 2025-01-30T17:12:44Z

discord/interactions/addConfigCommand.ts

@@ -288,7 +288,7 @@ export const handleConfigConfirmCommand = async (
      discordServerId: interaction.guildId,
      starkyModuleConfig: currentConfig.moduleConfig,
      discordRoleId: currentConfig.roleId,
-      starknetNetwork: currentConfig.network,
+      //starknetNetwork: currentConfig.network,


Why ? And don't comment code you don't use anymore, just delete. Else we will end up with comments everywhere

Marchand-Nicolas · 2025-01-30T17:12:51Z

discord/interactions/debugUser.ts

@@ -66,7 +66,7 @@ export const handleDebugUserCommand = async (
    const network = discordMember.starknetNetwork as NetworkName;
    const discordServerConfigs = await DiscordServerConfigRepository.findBy({
      discordServerId: guildId,
-      starknetNetwork: network,
+      //starknetNetwork: network,


Marchand-Nicolas · 2025-01-30T17:12:57Z

indexer/indexer.ts

@@ -22,14 +22,14 @@ const launchIndexers = () => {
  const blockStack = new BlockStack();
  // For each network, launch an indexer
  for (let network of networks) {
-    if (network.indexer === true) {
+    /*if (network.indexer === true) {


Marchand-Nicolas · 2025-01-30T17:13:03Z

indexer/processBlocks.ts

@@ -28,7 +28,7 @@ const processBlocks = async (stack: BlockStack) => {
      if (!walletAddress) continue;
      const discordConfigs = await DiscordServerConfigRepository.findBy({
        discordServerId: member.discordServerId,
-        starknetNetwork: networkName,
+        //starknetNetwork: networkName,


Marchand-Nicolas · 2025-01-30T17:14:14Z

utils/discord/refreshRoles.ts

@@ -58,7 +58,7 @@ export const refreshDiscordMemberForAllConfigs = async (
  const network = discordMember.starknetNetwork as NetworkName;
  const discordServerConfigs = await DiscordServerConfigRepository.findBy({
    discordServerId: discordMember.discordServerId,
-    starknetNetwork: network,
+    // starknetNetwork: network,


sotoJ24 · 2025-01-30T18:47:52Z

I have already fixed, could you check again please

Marchand-Nicolas

Still a lot of problems.
The code is not really clean...
You should install a linter, and take time to read your code. You'll learn a lot if you try reviewing it yourself before pushing.
I haven't tested the feature yet, will do it asap

Marchand-Nicolas · 2025-01-31T11:43:59Z

.env.local.sample

@@ -1,21 +0,0 @@
-NEXT_PUBLIC_DISCORD_CLIENT_ID=xxx


Please don't touch this file.
Eqverytime, before requesting a review, please check here #77 that you didn't do anything wrong

Marchand-Nicolas · 2025-01-31T11:49:00Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+  sepolia: string;
+};
+
+const chainIds: ChainIds = {


Why adding this variable ? It's not used

Marchand-Nicolas · 2025-01-31T11:49:02Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+  signMessage: (message: any) => Promise<Signature>;
+};
+
+type StarknetAccount = {


Do not define types here, it's not clean. If you want to define types (other than Props), define them in the folder types

Marchand-Nicolas · 2025-01-31T11:49:28Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+
+  useEffect(() => {
+    const fetchNetworkConfig = async () => {
+      const response = await fetch("/configs/networks.json");


If you want to access the config, just import it, do not fetch it

Marchand-Nicolas · 2025-01-31T11:50:05Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

-      };
-      const signature = await account.signMessage(messageCopy);
-      await verifySignature(signature);
+      let signature;


this variable is not used

Marchand-Nicolas · 2025-01-31T11:50:19Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+          domain: { ...messageToSign.domain, chainId },
+        };
+        signature = messageCopy;
+      }
    } catch (e: any) {
      WatchTowerLogger.error(e.message, e);
    }
  }, [account, verifySignature, chainId]);


React Hook useCallback has an unnecessary dependency: 'verifySignature'. Either exclude it or remove the dependency array.

Marchand-Nicolas · 2025-01-31T11:53:21Z

Also there are still conflicts, so I can't merge

Marchand-Nicolas · 2025-01-31T17:49:25Z

Just tested, I see object object instead of the address, and it doesn't open my freighter wallet to accept connection.
Then, after signing with freighter the website does nothing. It should verify the signature like on starknet.

Also, as you didn't pull the changes made by the other contributors you cannot test.
Please TEST! That way you'll understand what you do and avoid errors.
Have you tried actually running the project ?

sotoJ24 · 2025-01-31T18:08:15Z

Hi Marchand-Nicolas, I have run the project before, but it shows me errors from the last merges, may be the conflicts come because of it, now I will clean the code, sorry I am learning, it's my first time on how to apply the connection wallet for Stellar, I just want learn and help, regards

Marchand-Nicolas · 2025-02-04T11:24:14Z

.env.local.sample

@@ -1,21 +0,0 @@
-NEXT_PUBLIC_DISCORD_CLIENT_ID=xxx


Please do not delete this file

Marchand-Nicolas · 2025-02-04T11:24:48Z

configs/chainAliasByNetwork.json

@@ -1,5 +1,7 @@
 {
  "goerli": ["SN_GOERLI", "0x534e5f474f45524c49"],
  "sepolia": ["SN_SEPOLIA", "0x534e5f5345504f4c4941"],
-  "mainnet": ["SN_MAIN", "0x534e5f4d41494e"]
+  "mainnet": ["SN_MAIN", "0x534e5f4d41494e"],
+  "stellar-mainnet": ["SN_STELLAR_MAIN", "0x534e5f53544c4c41524d41494e"],


Rename ST_MAIN and ST_TEST instead of SN_xxx for Stellar

Marchand-Nicolas · 2025-02-04T11:25:16Z

configs/networks.json

+  {
+    "name": "stellar-mainnet",
+    "url": "stellar-mainnet-url",
+    "indexer": true,


Set indexer to false for stellar because we currently don't have one for these networks

Marchand-Nicolas · 2025-02-04T11:25:30Z

db/entity/DiscordServerConfig.ts

@@ -16,7 +16,12 @@ export class DiscordServerConfig {
  discordServerId: string;

  @Column()
-  starknetNetwork: "goerli" | "mainnet" | "sepolia";
+  starknetNetwork:


Marchand-Nicolas · 2025-02-04T11:26:00Z

package.json

@@ -14,6 +14,8 @@
    "@apibara/protocol": "^0.4.8",
    "@apibara/starknet": "^0.3.0",
    "@discordjs/rest": "^2.1.0",
+    "@stellar/freighter-api": "^4.0.0",


The imports look good too well done

Marchand-Nicolas · 2025-02-04T11:26:37Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+  signMessage: (message: any) => Promise<Signature>;
+};
+
+type StarknetAccount = {


Please apply this comment https://github.com/nmalzieu/starky/pull/77/files#r1937105706

Marchand-Nicolas · 2025-02-04T11:26:55Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+
+  useEffect(() => {
+    const fetchNetworkConfig = async () => {
+      const response = await fetch("/configs/networks.json");


Please apply this comment https://github.com/nmalzieu/starky/pull/77/files#r1937107018

sotoJ24 · 2025-02-06T03:42:13Z

On it

sotoJ24 · 2025-02-10T05:55:05Z

Done, please review

Marchand-Nicolas

If am trying to connect to stellar, but: the button should be "Connect your Stellar wallet" instead of starknet. Also, when I click it, it opens the popup to connect to starknet, instead of stellar

Marchand-Nicolas · 2025-02-13T12:51:14Z

pages/verify/[discordServerId]/[discordMemberId]/[customLink].tsx

+  );
+
+  useEffect(() => {
+    const fetchNetworkConfig = async () => {


Don't fetch the config. Import it

sotoJ24 · 2025-02-18T18:41:25Z

Let me fix it

Marchand-Nicolas

Remove this step for stellar. You can add a "connectionNeeded" parameter to do so in configs/networks.json. Because we don't need to accept any connection. We can directly sign

sotoJ24 added 7 commits January 26, 2025 14:29

Merge pull request #1 from nmalzieu/main

ce7e68b

Merge last changes

Update networks.json

9b121d3

Update chainAliasByNetwork.json

b286b8b

Update [customLink].tsx

8f5e339

Create .env.local

0573a52

Update .env.local

d5a3d98

Create package-lock.json

e55abc9

sotoJ24 changed the title ~~Fix issue 52~~ Feature: Implement wallet connection for Stellar Jan 27, 2025

Marchand-Nicolas requested changes Jan 27, 2025

View reviewed changes

sotoJ24 added 4 commits January 27, 2025 08:05

Delete .env.local.sample

19945c2

Merge branch 'nmalzieu:main' into fix-issue-52

ba6ccf6

branch main commits to the master branch

b7f393b

Feature: Implement wallet connection for Stellar

4cfd44a

Marchand-Nicolas requested changes Jan 30, 2025

View reviewed changes

fix: remove the lines of documentation

b9f9769

Marchand-Nicolas requested changes Jan 31, 2025

View reviewed changes

sotoJ24 added 3 commits February 3, 2025 23:34

Merge branch 'main' into fix-issue-52

5c2094d

fix: merge conflicts

98b2469

Merge branch 'main' into fix-issue-52

2f36dc2

Marchand-Nicolas requested changes Feb 4, 2025

View reviewed changes

fix: code clean and value changes

cd3fc42

Marchand-Nicolas requested changes Feb 13, 2025

View reviewed changes

sotoJ24 added 2 commits February 19, 2025 18:52

fix: branch conflicts

461f880

fix: branch conflicts

c3dd66a

Marchand-Nicolas requested changes Feb 20, 2025

View reviewed changes

Feature: Implement wallet connection for Stellar #77

Are you sure you want to change the base?

Feature: Implement wallet connection for Stellar #77

Conversation

sotoJ24 commented Jan 27, 2025 • edited Loading

7️⃣ Create a Pull Request for issue #52 (PR)

! Changes

Marchand-Nicolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sotoJ24 commented Jan 30, 2025

Marchand-Nicolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sotoJ24 commented Jan 30, 2025

Marchand-Nicolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marchand-Nicolas commented Jan 31, 2025

Marchand-Nicolas commented Jan 31, 2025

sotoJ24 commented Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sotoJ24 commented Feb 6, 2025

sotoJ24 commented Feb 10, 2025

Marchand-Nicolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sotoJ24 commented Feb 18, 2025

Marchand-Nicolas left a comment

Choose a reason for hiding this comment

sotoJ24 commented Jan 27, 2025 •

edited

Loading

sotoJ24 commented Jan 31, 2025 •

edited

Loading