QuantumMechanics / NEM-sdk

NEM Developer Kit for Node.js and the browser
MIT License
137 stars 82 forks source link

Error? in prepare("mosaicTransferTransaction") result fee is null. #36

Open scrpgil opened 6 years ago

scrpgil commented 6 years ago

Hi, I am always indebted to NEM-sdk.

I noticed that the result of calling nem.model.transactions.prepare ("mosaicTransferTransaction") is Fee null.

var transactionEntity = nem.model.transactions.prepare("mosaicTransferTransaction")(common, transferTransaction, mosaicDefinitionMetaDataPair, nem.model.network.data.testnet.id);
// !transactionEntity.fee == null;

Moreover, I think that cause is in the following code of the calculateMosaics function. In the following code, is not it possible to get initialSupply correctly? fees.js#124

        let supply = mosaicDefinitionMetaDataPair.supply; //

I can get initialSupply if I change it like this, fee will not be undefine.

        let initialSupplyProperties = Helpers.grep(mosaicDefinitionMetaDataPair.mosaicDefinition.properties, function(w) {
            return w.name === "initialSupply";
        });
        let supply = initialSupplyProperties.length === 1 ? (initialSupplyProperties[0].value) : 0;

I will separately send you a pull request, please check. thank, you.

*Also, I am bad at English. I am using Google Translate to write this issue.

collectivemass commented 6 years ago

Can confirm that this error blocks mosaic transfers and that the proposed fix fixes the issue. Thanks, @scrpgil !

gdhern commented 6 years ago

@scrpgil I was encountering this issue as well, I added in the fix and was able to continue. When the request is sent I get a bad request error saying that the fee information can not be found. Did you encounter that problem as well?

scrpgil commented 6 years ago

@gdhern Is it probrem FAILURE_INSUFFICIENT_FEE? If you see a message like the one below, I think it is a similar problem to this issue.

 { innerTransactionHash: {},
        code: 17,
        type: 1,
        message: 'FAILURE_INSUFFICIENT_FEE',
        transactionHash:
         { data: '4e900d75f59cf3158319ce6a7942446870d8a5436f1c2479e77779d64910259d' } }

If possible, please tell me what kind of error message returned. I may also be able to find out the cause of the error.

gdhern commented 6 years ago

@scrpgil I get the following error. I just changed the actual namespaceid and name in the message. I log the prepared transaction object and the fee appears there, but send function leads to this error. ERROR { code: 0, data: { timeStamp: 93476314, error: 'Bad Request', message: 'unable to find fee information for \'id:name\'', status: 400 } }

scrpgil commented 6 years ago

@gdhern I'm sorry. I examined the error, but I could not find the cause. Please compare if there is no difference from your request as I write the request when I said well.

// Request to http://50.3.87.123
{ type: 257,
        version: -1744830462,
        signer: 'd914bf22b9d070f9a93b43d6c0141ccdb4e5387388197c0ecde135dc02e0d757',
        timeStamp: 93493676,
        deadline: 93497276,
        recipient: 'TAMH452PDTXYANJFDWXVPXFHSAEBN23Z7NKPWJEP',
        amount: 1000000,
        fee: 150000,
        message: { type: 1, payload: '74657374' },
        mosaics:
         [ { mosaicId: { namespaceId: 'nem', name: 'xem' }, quantity: 1000000 },
           { mosaicId: { namespaceId: 'greeting', name: 'ya' }, quantity: 10000000 } ] }

Also, I can put definition information of mosaic that I sent.

//mosaicDefinition
{
  "nem:xem": {
    "mosaicDefinition": {
      "creator": "3e82e1c1e4a75adaa3cba8c101c3cd31d9817a2eb966eb3b511fb2ed45b8e262",
      "description": "reserved xem mosaic",
      "id": {
        "namespaceId": "nem",
        "name": "xem"
      },
      "properties": [
        {
          "name": "divisibility",
          "value": "6"
        },
        {
          "name": "initialSupply",
          "value": "8999999999"
        },
        {
          "name": "supplyMutable",
          "value": "false"
        },
        {
          "name": "transferable",
          "value": "true"
        }
      ],
      "levy": {}
    }
  },
  "greeting:ya": {
    "mosaicDefinition": {
      "creator": "d914bf22b9d070f9a93b43d6c0141ccdb4e5387388197c0ecde135dc02e0d757",
      "description": "Ya is greeting token.",
      "id": {
        "namespaceId": "greeting",
        "name": "ya"
      },
      "properties": [
        {
          "name": "divisibility",
          "value": "6"
        },
        {
          "name": "initialSupply",
          "value": "9000000000"
        },
        {
          "name": "supplyMutable",
          "value": "true"
        },
        {
          "name": "transferable",
          "value": "true"
        }
      ],
      "levy": {}
    }
  }
}
maxp commented 6 years ago

Confirm the bug.

gdhern commented 6 years ago

@scrpgil There's no difference between the object you posted and the one that appears in my log.

scrpgil commented 6 years ago

@gdhern Thank you for confirmation! I think that the difference is the cause of the error. Please copy & paste that log. Also, you may find out the cause if you tell me the node you are connected to.

gdhern commented 6 years ago

@scrpgil And thank you for helping me out! I have been using nem.model.objects.create("endpoint")(nem.model.nodes.defaultTestnet, nem.model.nodes.defaultPort)

and for the network

nem.model.network.data.testnet.id

this is the console log of the prepared object

{ type: 257, version: -1744830462, signer: '56a6e5dc2b756818e40c84c501780948f322e21b591413afe770e8d29b00ea66', timeStamp: 93583848, deadline: 93587448, recipient: 'TBCI2A67UQZAKCR6NS4JWAEICEIGEIM72G3MVW5S', amount: 1000000, fee: 200000, message: { type: 1, payload: '5472616e73616374696f6e20666f7220323030312053656174746c65204d6172696e657273205369676e6564204261736562616c6c' }, mosaics: [ { mosaicId: [Object], quantity: 3000000 }, { mosaicId: [Object], quantity: 1000000 } ] }

scrpgil commented 6 years ago

@gdhern Thank you!

I looked at the log. There are several suspicious places in the sending address.

  1. The signer address has only 0 xem. Without xem you can not pay the fee required for sending transactions.
// call
nem.com.requests.account.data(endpoint , "TCRK3WDL2Q33E3FQXWDZJC7TGF7CEVQPQNFBGMB6").then(...)
// response 
{ data:{
   account:{
     address:"TCRK3WDL2Q33E3FQXWDZJC7TGF7CEVQPQNFBGMB6",
     balance: 0, ★0 xem
  ~~~~
}
  1. The signer address has only xem mosaic. Because I do not have a mosaic other than xem, I can not send that mosaic.
// call
nem.com.requests.account.mosaics.owned(endpoint , "TCRK3WDL2Q33E3FQXWDZJC7TGF7CEVQPQNFBGMB6").then(...)
// response
data:[  {
   mosaicId: {namespaceId: "nem", name: "xem"},  
   quantity:0}
]

Why do not you try again after depositing xem and mosaic at the sending address?

QuantumMechanics commented 6 years ago

Hi,

Sorry for late reply.

We calculate the fee from the current supply, not the initial supply.

It can't be 0 like in your pull because it will return FAILURE_INSUFFICIENT_FEE.

Objects must contain the current supply as an extra supply property, like this:

{
    "mosaic:mymosaic1": { mosaicDefinition: {…}, supply: 8999900 },
    "mosaic:mymosaic2": { mosaicDefinition: {…}, supply: 10009999 }
}

Current supply can be 8999900 and initialSupply be 8999999999. The quantity change of a mosaic is not reflected into the initialSupply property in mosaicDefinition.

I had to make this change recently, users got insufficient fee error in Nano because I was using initialSupply and not the current supply.

maxp commented 6 years ago

I've found that parameter digged deeply in the code. Its good idea to mention .supply variable in that example https://github.com/QuantumMechanics/NEM-sdk/blob/master/examples/nodejs/mosaicTransfer.js

scrpgil commented 6 years ago

@maxp Thanks for giving me a hint! I rewrote the example.

// Include the library
var nem = require("../../build/index.js").default;

~~~~~~~~
nem.com.requests.namespace.mosaicDefinitions(endpoint, mosaicAttachment2.mosaicId.namespaceId).then(function(res) {

        // Look for the mosaic definition(s) we want in the request response (Could use ["eur", "usd"] to return eur and usd mosaicDefinitionMetaDataPairs)
        var neededDefinition = nem.utils.helpers.searchMosaicDefinitionArray(res.data, ["eur"]);

        // Get full name of mosaic to use as object key
        var fullMosaicName  = nem.utils.format.mosaicIdToName(mosaicAttachment2.mosaicId);

        // Check if the mosaic was found
        if(undefined === neededDefinition[fullMosaicName]) return console.error("Mosaic not found !");

        // Set eur mosaic definition into mosaicDefinitionMetaDataPair
        mosaicDefinitionMetaDataPair[fullMosaicName] = {};
        mosaicDefinitionMetaDataPair[fullMosaicName].mosaicDefinition = neededDefinition[fullMosaicName];

        // ★Get current supply with nem:xem
        nem.com.requests.mosaic.supply(endpoint, nem.utils.format.mosaicIdToName(mosaicAttachment.mosaicId)).then(function(res) {
                mosaicDefinitionMetaDataPair[nem.utils.format.mosaicIdToName(mosaicAttachment.mosaicId)].supply = res.supply;
                // ★Get current supply with eur:usd
                nem.com.requests.mosaic.supply(endpoint, fullMosaicName).then(function(res) {
                        mosaicDefinitionMetaDataPair[fullMosaicName].supply = res.supply;
                        // Prepare the transfer transaction object
                        var transactionEntity = nem.model.transactions.prepare("mosaicTransferTransaction")(common, transferTransaction, mosaicDefinitionMetaDataPair, nem.model.network.data.testnet.id);

                        // Serialize transfer transaction and announce
                        nem.model.transactions.send(common, transactionEntity, endpoint);
                }, function(err) {
                console.error(err);
                });
        }, function(err) {
        console.error(err);
        });

}, function(err) {
    console.error(err);
});

@QuantumMechanics Thank you for giving me a reply! Please let me question as many as 2 points.

  1. "nem.com.requests.mosaic.supply()" is not listed in the README but can I use it?
  2. nem-library-ts uses initialSupply. Is this a mistake? https://github.com/aleixmorgadas/nem-library-ts/blob/d8df40c5a1f8b2cc858e416d7c0f9801d21e6d2d/src/models/transaction/TransferTransaction.ts#L209 I do not think it is directly related, but I'd like to ask your opinion.
flute commented 6 years ago

@scrpgil Thanks for your example, It truly works. But i‘m still in confusion. When sending a mosaic transaction, why need to send 1 nem both? finally i cost 1 nem + fee.

evias commented 6 years ago

no it doesnt cost you 1 XEM, in case of mosaic transfers, the transaction.amount field is used as a multiplier for the attachments. You only pay the fee + the mosaics attached (in case transaction.amount is not 0)

This means if you put 3 there, it would affect the balance 3 times with said attachments.

evias commented 6 years ago

@scrpgil

  1. "nem.com.requests.mosaic.supply()" is not listed in the README but can I use it?

Yes this method works and returns exactly the supply data that you need.

nem-library-ts uses initialSupply. Is this a mistake? https://github.com/aleixmorgadas/nem-library-ts/blob/d8df40c5a1f8b2cc858e416d7c0f9801d21e6d2d/src/models/transaction/TransferTransaction.ts#L209

Yep as you mention it, with mutableSupply mosaics this would be a bug, probably resulting in INSUFFICIENT_FEE as well. You might want to file that bug over at nem-library-ts, can't tag aleix in this discussion

truedat101 commented 5 years ago

For noobs it would be nice to fix the example code provided with the one liner:

mosaicDefinitionMetaDataPair[fullMosaicName].supply = neededDefinition[fullMosaicName].properties[1].value;

around this line: https://github.com/QuantumMechanics/NEM-sdk/blob/master/examples/nodejs/mosaicTransfer.js#L57

Until discovering this thread on github, I verified I could set the fee manually and get a successful transaction. However, glad to find a solution on the forums.

evias commented 5 years ago

I believe that this fix would not be enough though, because that will work fine only with non-mutable mosaics.

With mutable mosaics it is important to use the mosaics.supply() endpoint.

truedat101 commented 5 years ago

Thank you for the clarification. Interesting, I'll try that out in my code tonight. I guess to say, the examples should be functional (work without modifications other than using valid wallet and destination address) and instructive (there are reasonably good comments in the examples). I forgot about mutability! We were having a discussion here on this forum topic related: https://forum.nem.io/t/nem-sdk-js-mosaic-transfertransaction-doesnt-compute-fee/21106