search

oasisprotocol / sapphire-paratime

Oasis Sapphire - the confidential EVM-compatible ParaTime for the Oasis Network
https://oasisprotocol.org/sapphire
Apache License 2.0
39 stars 27 forks source link

clients/viem: use interval for routine fetching, unref intervalId #383

Closed CedarMist closed 2 months ago

CedarMist commented 2 months ago

Provide actual fix for problem inside #379

netlify[bot] commented 2 months ago

Deploy Preview for oasisprotocol-sapphire-paratime ready!

Name Link
Latest commit c34b198643f11384c309b998d41f8a349dec3fc1
Latest deploy log https://app.netlify.com/sites/oasisprotocol-sapphire-paratime/deploys/66e0ab19903be200092e8191
Deploy Preview https://deploy-preview-383--oasisprotocol-sapphire-paratime.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

CedarMist commented 2 months ago

This won't work in browsers.

We can do something like:

if( typeof x === 'object' && 'unref' in x && typeof x.unref == 'function' )

or just

if( typeof x.unref === 'function' )

?

Likewise we're not doing any in-browser testing for packages which will primarily be used in browsers, so none of this would've been caught in CI.

CedarMist commented 2 months ago

Will find a way of linting the TS code for NodeJS vs WebBrowser compatibility.

CedarMist commented 2 months ago

Have logged a ticket/discussion with Viem w.r.t async serializers, which would entirely avoid this hacky workaround. I should've done this earlier.

https://github.com/wevm/viem/discussions/2696

aefhm commented 2 months ago

A standalone NodeJS script reproduces the exit error pretty easily, but reproducing it in vitest has been tricky. Test harness is not waiting for the Node process, and does not hang which is the behavior we want to reproduce in the failing case. We could use a test script and assert that this exits within 1000 milliseconds though.

const transport = sapphireHttpTransport();
const chain = sapphireLocalnet;
const publicClient = createPublicClient({ chain, transport });
defineChain({
    id: 0x5afd,
    name: "Oasis Sapphire Localnet",
    network: "sapphire-localnet",
    nativeCurrency: { name: "Sapphire Local Rose", symbol: "TEST", decimals: 18 },
    rpcUrls: {
        default: {
            http: ["http://localhost:8545"],
        },
    },
    serializers: {
        transaction: createSapphireSerializer(publicClient)
    },
    testnet: true,
});
CedarMist commented 2 months ago

Ok, I can reproduce this. It appears that my unref fix works correctly. I will see about how to add a test.

Have added a timeout test, to verify that your example script exits after 2 seconds.