Zilliqa / zilliqa-js

JavaScript SDK for Zilliqa blockchain
https://www.npmjs.com/package/@zilliqa-js/zilliqa
GNU General Public License v3.0
131 stars 74 forks source link

Ability to disable automatic signing when using a provider #140

Closed krisboit closed 3 years ago

krisboit commented 5 years ago

Is your feature request related to a problem? Please describe. When using a provider for creating an Zilliqa instance there should be a way to disable automatic signing. For Moonlet Wallet we have a provider but as we don't pass the privateKey into the Zilliqa instance the sign process is failing.

A Moonlet Example:

const zilliqa = new Zilliqa('', moonlet.providers.zilliqa);

zilliqa.blockchain.createTransaction(...); // this will fail since it doesn't have the actual account

// a workaraound
zilliqa.blockchain.signer = zilliqa.contracts.signer = {
    sign: m => m
};

Describe the solution you'd like I don't have something in mind, but maybe it would be a good idea to disable automatic signing if a provider is set.

neeboo commented 5 years ago

You can add a another blockchain method like:

async createTransactionWithSigner(tx:Transaction,signer:Wallet){
   if(!signer){throw...}
   const signed=await signer.sign(tx);
   this.provider.send(RPCMethod.createTransaction,{
        ...signed.txParams,
        priority: signed.toDS,
      }
  ...
}
krisboit commented 5 years ago

@neeboo For Wallets extensions the signing is happening inside this.provider.send(), and it's happening only if the user confirms it.

I would suggest to have a single method createTransaction and do the magic in @sign decorator. There we could do a check based on a field on the provider. Example:

if (this.provider.autoSignOnCreateTransaction) {
// ... do the signing
}

Also by adding a new method as you suggested will include an overhead since all the methods that rely on createTransaction() will have to be duplicated...

krisboit commented 5 years ago

Even if you don't agree with my suggestion, i think that the signer should be part of the provider, and the signing logic should sit in @sign decorator, because this issue impacts all method using @sign. So this way we can keep only one version of methods that basically do the same thing.

iantanwx commented 5 years ago

@krisboit I agree. Let us think about how to fix this. Sorry for the late reply.

neeboo commented 5 years ago

@neeboo For Wallets extensions the signing is happening inside this.provider.send(), and it's happening only if the user confirms it.

I would suggest to have a single method createTransaction and do the magic in @sign decorator. There we could do a check based on a field on the provider. Example:

if (this.provider.autoSignOnCreateTransaction) {
// ... do the signing
}

Also by adding a new method as you suggested will include an overhead since all the methods that rely on createTransaction() will have to be duplicated...

So the extension use provider as wallet signer and request manager at the same time?

krisboit commented 5 years ago

@neeboo Yes, Moonlet is using the provider to intercept all the calls made by zilliqa js library. Before doing the actual call it does some logic.

For CreateTransaction call, it overwrites some fields: version, pubKey, nonce, then it signs the transaction and then the transaction is broadcasted.

This is how we implemented, because we were thinking that developed that use ZilliqaJS with Moonlet toghether doesn't have to know too much about what's behind. The only major difference should be the initialization of Zilliqa class: const zilliqa = new Zilliqa('', window.moonlet.providers.zilliqa).

Even if we already did our implementation we are open to discuss and find the best solution :)