Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the big clean up refactor! #47

Open
12 of 15 tasks
spencercap opened this issue Oct 27, 2022 · 6 comments
Open
12 of 15 tasks

the big clean up refactor! #47

spencercap opened this issue Oct 27, 2022 · 6 comments
Assignees

Comments

@spencercap
Copy link
Member

spencercap commented Oct 27, 2022

lets...

  • modularize functions / lib abilities into individual categorized files / folders
  • pair down redundant/duplicate methods (possibly: sendAtomicTransaction and sendTransaction, and check various account info gets)
  • make private + public class methods
  • optimize for built lib size (treeshake + bundle correctly in esbuild)
  • dont include all of algosdk as .sdk ? @youraerials thoughts?
  • make signing methods Providers (inkey, pera, algosigner, local, etc) see notes here
    • by modularizing wallet connect we can remove the mp3's as .ts files shim too + the request animation frame, silent audio hack for keeping the web socket open while backrounded

Delwin's additions:
These issues should be closed with this refactor, as they significantly impact the structure or default behavior of Algonaut:

And lastly:

@enceladus
Copy link
Collaborator

enceladus commented Nov 2, 2022

On the last point — I think we definitely need to figure out how to handle different signing methods better, but the class-per-provider method seems a little verbose to me. To recap (from the notes linked above):

import { WalletConnectProvider, Algonaut } from '@thencc/algonautjs'
const walletConnectProvider = new WalletConnectProvider({...});
const algonaut = new Algonaut();
algonaut.useSigningMethod(walletConnectProvider);

We could have the providers encapsulate Algonaut themselves:

// if doing things with local accounts
import { Algonaut } from '@thencc/algonautjs'
const algonaut = new Algonaut()
const account = algonaut.recoverAccount(mnemonic)

// if using Inkey only
import { Inkey } from '@thencc/algonautjs'
const inkey = new Inkey() // this just returns an Algonaut instance configured with Inkey
inkey.sendAlgo(...) // = algonaut.sendAlgo

That would be nice if you're trying to use multiple signing methods at once, but it also might confuse people?

I also like having each provider separate, but accessible from the Algonaut class:

import { Algonaut } from '@thencc/algonautjs'
const algonaut = new Algonaut()
algonaut.inkey.use()
algonaut.inkey.connect()
algonaut.sendAlgo()

Ultimately I guess it doesn't really matter, but the point here is standardization (instead of inconsistently named methods depending on provider) and choosing the best experience for devs. Any opinions @spencercap @youraerials @lanidelrey ?

@spencercap
Copy link
Member Author

yeh, i hear you on the first bit being verbose.

how about taking some inspo from how vue3 works? they are functional vs class based.

import { createApp } from 'vue';
const app = createApp(App);

// router (could be in router.ts)
import { createRouter } from 'vue-router';
const router = createRouter({
  routes: [
    {
      path: '/',
      component: Home
    },
    ...
    ]
});

// plugins for vue app
import router from './router';
app.use(router);

// some more plugins etc etc...

(from here)

after vue calls app.user(router), then this.$route and this.$router are available in components.


algonaut can do the same, populating algonaut.inkey after a algonaut.use(inkey)
something like:

import { createAlgonaut, createInkeyPlugin } from '@thencc/algonautjs';

// algonaut
const algonaut = createAlgonaut({ ... });

// inkey
const inkeyPlugin = createInkeyPlugin({ options: ... });
algonaut.use(inkeyPlugin);

// in use...
await algonaut.inkey.connect(); 
await algonaut.sendAlgo({...}); // hm... how does algonaut know which signing method to use in the design... 

@spencercap
Copy link
Member Author

spencercap commented Nov 3, 2022

(this is more of an inkey idea/rec but relevant here too)

hm... thinking about the future where inkey doesnt just support algorand, but other chains too.
it's almost like inkey will need a inkey-client library which handles all post message comms. currently this lives within algonautjs, but one day (soon) we'll want to send inkey some eth txn base64'd to sign which might look like...

import { createInkeyClient } from 'inkey-client'; // think, socketio setup

// mounts inkey iframe in dapp DOM + sets up post message channel
const inkeyClient = await createInkeyClient({ ... }); // can use await in functional creation approach

// ALGO: make a txn in algonautjs or js algosdk
let txn1 = algonaut.atomicSendAlgo({
  to: '123...',
  amount: 3
}).toStr();
// then, send it to inkey 
let data = {
    source: 'ncc-inkey-client',
    uuid: new Date(),
    async: true,
    type: 'sign-txns', // same for algorand or eth
    payload: {
        chain: 'algorand',
        // array of base64 encoded unsigned transactions
        txns: [ txn1, txn2, txn3 ] 
    }
};
let res = await inkeyClient.send(data);

// ETH: then maybe request an eth signing from inkey
let data2 = {
    source: 'ncc-inkey-client',
    uuid: new Date(),
    async: true,
    type: 'sign-msg',
    payload: {
        chain: 'ethereum', // tells ncc-inkey-wallet to grab the user's ethereum wallet!
        msg: 'something to sign'
    }
};
let res2 = await inkeyClient.send(data2);

@spencercap
Copy link
Member Author

@enceladus
would you say we should pluck any of these out into their own new issues?
most seem pretty dated to me

@spencercap
Copy link
Member Author

spencercap commented Jun 6, 2023

make private + public class methods

^ do we really need this?


optimize for built lib size (treeshake + bundle correctly in esbuild)

i think this is happening already...


dont include all of algosdk as .sdk ? @youraerials thoughts?

lets keep the whole algosdk for the time being. it's useful when pushing the boundaries


modularize functions / lib abilities into individual categorized files / folders

this could be useful still! pluck this for a new issue?


Update & expand ALL documentation accordingly (do we want to use VuePress 2?) #52

what about this one? @enceladus

@enceladus
Copy link
Collaborator

enceladus commented Jun 8, 2023

The only one worth keeping for now is redoing the documentation. I'll make a new issue for that.

Refactoring the file into several different files is just busy work for now, imo.

What about: #12 #49 #32 ? @spencercap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants