tuler / deroll

TypeScript framework for Cartesi applications
https://deroll.vercel.app
MIT License
18 stars 13 forks source link

Add contracts to Deroll #62

Closed sandhilt closed 7 months ago

sandhilt commented 8 months ago

Some features for Deroll

changeset-bot[bot] commented 8 months ago

⚠️ No Changeset found

Latest commit: 9892f174616eb2dc56268c96cd39aa36c3b5f352

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

tuler commented 8 months ago

Thanks for opening a PR. Is it ready for review?

tuler commented 8 months ago

Please organize the commits in a way that helps the review process.

sandhilt commented 8 months ago

Please organize the commits in a way that helps the review process.

Ok. I will rebase that. Thanks for quickly response.

sandhilt commented 8 months ago

Hello @tuler, please feel free for send comments and review this PR. If need more fixes, please, let me know.

tuler commented 7 months ago

package @types/node is missing in devDependencies of wallet package

tuler commented 7 months ago

I see a bunch of typescript errors

sandhilt commented 7 months ago

Hello @tuler, I change some things based on your comments and adjust with @fabiooshiro. Please, let me know if it's better or needs more adjustments.

tuler commented 7 months ago

I still don't understand why we need the design of DepositOperation, DepositArgs, some of the error classes. I think it's overly complicated, and does not follow the design of deroll, which is centralized on AdvanceRequestHandler.

What we should have with the addition of ERC721 and ERC1155 should be built on top of that concept of AdvanceRequestHandler.

The wallet export a handler to be attached to the pipeline of deroll. Internally it should follow the same concept. Internally the WalletImpl handler should be something as simple as:

    public handler: AdvanceRequestHandler = async (data) => {
        const msgSender = getAddress(data.metadata.msg_sender);
        const handler = handlers[msgSender];
        if (handler) {
            return handler(data);
        }
        return "reject";
    };

And the WalletImpl class should have something like:

    private handlers: Record<Address, AdvanceRequestHandler>;

    constructor() {
        this.handlers = {
            [etherPortalAddress]: etherHandler,
            [erc20PortalAddress]: erc20Handler,
            [erc721PortalAddress]: erc721Handler,
            [erc1155SinglePortalAddress]: erc1155SingleHandler,
            [erc1155BatchPortalAddress]: erc1155BatchHandler,
        };
    }

And each token module exports a handler function on its own.

sandhilt commented 7 months ago

I still don't understand why we need the design of DepositOperation, DepositArgs, some of the error classes. I think



And each token module exports a handler function on its own.

Ok, I can follow this pattern. And how I access field wallet from WalletImpl inside handler (method or function of module) based in AdvanceRequestHandler?