starknet-io / get-starknet

StarkNet wallet <-> dApp bridge
MIT License
156 stars 108 forks source link

v2 - silent-connect ("neverAsk") regression #166

Open avimak opened 1 year ago

avimak commented 1 year ago

v2 introduces a regression in the silent-connect (now "neverAsk") flow.

    // silently attempt to connect with a pre-authorized wallet
    useEffect(() => {
        // match the dapp with a wallet instance
        connect({ modalMode: "neverAsk" }).then(wallet => {
            // connect the dapp with the chosen wallet instance
            wallet?.enable().then(() => {
                const isConnected = !!wallet?.isConnected;
                setIsConnected(isConnected);
            });
        });
    }, []);

the code above works 100% of the time using v1.5, and it fails on v2.

I suspect a race because adding a timeout with delay=0 helps, i.e. -

    // silently attempt to connect with a pre-authorized wallet
    useEffect(() => {
        setTimeout(() => {
            // match the dapp with a wallet instance
            connect({ modalMode: "neverAsk" }).then(wallet => {
                // connect the dapp with the chosen wallet instance
                wallet?.enable().then(() => {
                    const isConnected = !!wallet?.isConnected;
                    setIsConnected(isConnected);
                });
            });
        }, 0); // <-- no real delay, just skip a cycle
    }, []);

to reproduce, simply use the first version (without a timeout), choose a wallet, approve the connection (aka "pre-authorize"), then refresh the page. do it with a visible browser-inspect-application tab displayed, you'll see the id being removed (ptobably because getAvailableWallets returns empty array).

one more thing - if you adding a timeout, the code works, but you'll see multiple storage ids, meaning there are more races in there.

janek26 commented 1 year ago

https://user-images.githubusercontent.com/6831124/205700453-d9239aee-1860-4637-86bf-ddaf2cea14a9.mp4

I was not able to reproduce this, using nextjs default boilerplate and the code you provided above Am I missing anything?

avimak commented 1 year ago

it reproduces also on the new example app (though not 100%). btw, another bug - I'm getting another wallet while having a last-wallet set (and pre-authorized/"enabled" in the wallet itself) in storage. see both mentioned issues in these 2 videos -

https://user-images.githubusercontent.com/2437994/205730407-a32ed51f-2649-4f2b-aee9-aeab442c9fb2.mov

https://user-images.githubusercontent.com/2437994/205730444-e5c1c4d1-6db5-4658-9fde-025a5d6f8f02.mov

lazarmitic commented 1 year ago

I'm also seeing something similar, after I connect the wallet and refresh the page and call connect with neverAsk option, function:

const availableWallets = scanObjectForWallets(
    windowObject,
    isWalletObject,
)

fails to find starknet object on window object. Should I add some kind of wait, so that starknet object is injected in window object before get-starknet tries to read it?