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

add_change_if_needed very slow #271

Closed
YFrendo opened this issue Nov 24, 2021 · 9 comments · Fixed by #214
Closed

add_change_if_needed very slow #271

YFrendo opened this issue Nov 24, 2021 · 9 comments · Fixed by #214
Milestone

Comments

@YFrendo
Copy link

YFrendo commented Nov 24, 2021

Hy everyone.

I try to implement the Nami Wallet on one of my website and so I use this library to generate transaction.
I have a performance issue with the function add_change_if_needed(), it took 50sc to be execute on google chrome.
The transaction is very simple :send X ADA to one address.
I'm using the 9.1.0 version full JS.
Here some code:

async function pay(addr, adaAmount){
    const cardano = window.cardano
    const protocolParameters = await getProtocolParameters()
    const lovelace = (parseFloat(adaAmount) * 1000000).toString()

    var ByteAddress = await window.cardano.getUsedAddresses();
    const paymentAddr = await S.Address.from_bytes(fromHex(ByteAddress[0])).to_bech32(); 

    const rawUtxo = await cardano.getUtxos()
    const utxos = rawUtxo.map(u => S.TransactionUnspentOutput.from_bytes(fromHex(u)))
    const outputs = S.TransactionOutputs.new()

    outputs.add(
        S.TransactionOutput.new(
            S.Address.from_bech32(addr),
            S.Value.new(
                S.BigNum.from_str(lovelace)
            )
        )
    )
    const output = S.TransactionOutput.new(
      S.Address.from_bech32(addr),
      S.Value.new(
          S.BigNum.from_str(lovelace)
      )
  )

    CoinSelection.setProtocolParameters(
        protocolParameters.minUtxo.to_str(),
        protocolParameters.linearFee.coefficient().to_str(),
        protocolParameters.linearFee.constant().to_str(),
        protocolParameters.maxTxSize.toString()
      );
      
    const selection = await CoinSelection.randomImprove(
      utxos,
      outputs,
      20,
      //protocolParameters.minUtxo.to_str()
    );

    const inputs = selection.input;

    const txBuilder = S.TransactionBuilder.new(
      protocolParameters.linearFee,
      protocolParameters.minUtxo,
      protocolParameters.poolDeposit,
      protocolParameters.keyDeposit,
      protocolParameters.maxValSize, 
      protocolParameters.maxTxSize
    );

   for (let i = 0; i < inputs.length; i++) {
       const utxo = inputs[i];
         txBuilder.add_input(
         utxo.output().address(),
         utxo.input(),
         utxo.output().amount()
      );
     }


    txBuilder.add_output(output);
    txBuilder.set_ttl(410021);

    txBuilder.add_change_if_needed(
        S.Address.from_bech32(paymentAddr)
      );
    

There is no error and the transaction is send and can be signed.
If someone can help me thanks

@vsubhuman vsubhuman added this to the 10.0.0 milestone Nov 24, 2021
@vsubhuman
Copy link
Contributor

Hi, @YFrendo ! Test version 10.0.0-beta.5 plz, when you have a chance, there's already a fix included there that improves the change-calculation performance. Note that there are breaking API changes and you gonna need to update how you create the transaction-builder, see example here:

https://github.com/Emurgo/yoroi-frontend/blob/f1affc9f90efdce2c5b7078b80a3add97d9499a3/packages/yoroi-extension/app/api/ada/lib/cardanoCrypto/rustLoader.js#L85-L95

https://github.com/Emurgo/yoroi-frontend/pull/2530/files#diff-ed6bb4dfff2837130faf834b694396f7fa1d92580769f858c53308df8fd6419dR85-R94

@YFrendo
Copy link
Author

YFrendo commented Nov 24, 2021

HI! Thanks for your answer I will probably wait for the 10.0.0 official release!
Thanks for your work and good luck!

@rooooooooob
Copy link
Contributor

@YFrendo just to confirm, you're using the asm.js not the wasm builds right? I believe the fix (#214) @vsubhuman is talking about was mostly an issue in how absurdly slow the cryptography crates we depend on can be when compiled to asm.js. I would highly recommend wasm over the asm.js if its possible for you as in general wasm is just so much faster than asm.js.

@YFrendo
Copy link
Author

YFrendo commented Nov 25, 2021

Hy!
Yeah I can confirm I'm using the asm.js I will try with general wasm but not sure if I can.
Thanks for your awnser !

@YFrendo
Copy link
Author

YFrendo commented Nov 26, 2021

Hy @rooooooooob !
I tried to use the wasm version but I can't import it inside a navigator: Failed to load module script: The server responded with a non-JavaScript MIME type of "application/wasm". Strict MIME type checking is enforced for module scripts per HTML spec.
I found on internet that I need to import it in a another way but I need to change the most part of the script (to much work) do you have a solution for this ? https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Module/imports

@josemmrocha
Copy link

I also have this exact problem. Using asm.js is too slow to have it in production, and using wasm (browser), returns the same error: Failed to load module script: The server responded with a non-JavaScript MIME type of "application/wasm".

In my case I am trying to use it in an Angular SPA.

@alice1989123
Copy link

I am using Next.JS 12 I had the same Issue with pure JS-version, then i changed to browser version and the problem was solved.

@CyberCyclone
Copy link

@YFrendo just to confirm, you're using the asm.js not the wasm builds right? I believe the fix (#214) @vsubhuman is talking about was mostly an issue in how absurdly slow the cryptography crates we depend on can be when compiled to asm.js. I would highly recommend wasm over the asm.js if its possible for you as in general wasm is just so much faster than asm.js.

Based on this does v10 have no performance improvement for asm? I'm using MeteorJS and at this point they seem to have no interest in supporting wasm.

@rooooooooob
Copy link
Contributor

rooooooooob commented Dec 16, 2021

The performance has been improved further via #286 which should help the asm.js situation a bit. Any signing done will still be incredibly slow via asm.js though unfortunately but the building of the initial tx should be better. That PR will be included in the release of 10.0.0.

edit: I guess this isn't that bad if you're just using it with CIP-30 and the wallet will be doing the signing.

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