1
\$\begingroup\$

I wrote a AWS lambda function which buys €X worth of some currency on an exchange. For this I first have to find out the current price (getAskPrice), then calculate the amount to buy and post the order (buyLimitOrder). I've implemented both operations as promises, buyLimitOrder gets the result of getAskPrice as one of the arguments. The result of buyLimitOrder (if successful) should be returned as the result of the lambda function.

Here's the relevant code fragment:

const getAskPrice = new Promise(function(resolve, reject){

        got(TICKER_URL).then(response => {
            let result = JSON.parse(response.body);
            let ask = Number(result.ask);
            if (!isNaN(ask)) {
                resolve(ask);
            } else {
                throw new Error("Could not parse the ask price from [" + BTCEUR_TICKER_URL + "] response:\n" + response.body);
            }
        }).catch(reject);
});

const buyLimitOrder = function(customerId, key, secret, value, askPrice) {

    let amount = value / askPrice;

    let roundedAmount = Math.ceil(amount * Math.pow(10,5)) / Math.pow(10,5);

    let order = createAuthData(customerId, key, secret);
    order.amount = roundedAmount;
    order.price = askPrice;
    return new Promise(function(resolve, reject) {
        console.log("Buying " + roundedAmount + " of BTC for price " + askPrice + " for total value of " + (roundedAmount * askPrice) + ".");
        got.post(BUY_LIMIT_ORDER_URL, {
            body : order,
            form: true
        }).then(response => {
            let result = JSON.parse(response.body);
            resolve(result);
        }).catch(reject);
    });
};

const getAskPriceAndBuyLimitOrder = function(customerId, key, secret, value) {
    return new Promise(function(resolve, reject) {
        getAskPrice
            .then(askPrice => buyLimitOrder(customerId, key, secret, value, askPrice).then(resolve).catch(reject))
            .catch(reject);
    });
};

exports.handler = (event, context, callback) => {
    doWithBitstampProperties()
        .then(bitstampProperties => 
            getAskPriceAndBuyLimitOrder(bitstampProperties.customerId, bitstampProperties.key, bitstampProperties.secret, VALUE)
                .then(result => callback(null, result))
                .catch(error => callback(error)))
        .catch(error => callback(error));
};

Full code can be found here.

This works, but I was not sure this is how promises should be chained. For instance, calling .then(resolve).catch(reject) on buyLimitOrder(...) does not feel right:

    return new Promise(function(resolve, reject) {
        getAskPrice
            .then(askPrice => buyLimitOrder(customerId, key, secret, value, askPrice).then(resolve).catch(reject))
            .catch(reject);
    });

Generally, the code does not feel elegant.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ I'm no JavaScript programmer, but I thought return GetAskPrice.then(askPrice => ...) worked the same way as your above code. Have you tried this? \$\endgroup\$
    – Peilonrayz
    Commented Sep 10, 2017 at 19:45

2 Answers 2

2
\$\begingroup\$

Removing Promise constructor wrappers

As @Peilonrayz mentioned in his comment, we can return the promises directly instead of wrapping them in Promise constructors.

From the MDN: Promise docs:

The constructor is primarily used to wrap functions that do not already support promises.

Since we are already returning promises (and not wrapping callbacks, for example) we can remove the constructors.

For example, doWithBitstampProperties:

const doWithBitstampProperties = function() {
    return new Promise(function(resolve, reject){
        if (BITSTAMP_PROPERTIES) {
            resolve(BITSTAMP_PROPERTIES);
        } else {
            decryptBitstampProperties().then(resolve).catch(reject);
        }
    });
};

Could be rewritten to return the promise directly:

const doWithBitstampProperties = function() {
  if (BITSTAMP_PROPERTIES) {
    return Promise.resolve(BITSTAMP_PROPERTIES);
  }

  return decryptBitstampProperties();
};

Or, using the conditional operator if you preferred to be more terse:

const doWithBitstampProperties = function() {
  return BITSTAMP_PROPERTIES
    ? Promise.resolve(BITSTAMP_PROPERTIES)
    : decryptBitstampProperties();
};

We can repeat this pattern for the other functions and reduce the noise that all the resolve/rejects added:

const AWS = require('aws-sdk');
const crypto = require('crypto');
const got = require('got');

const CURRENCY_PAIR = 'btceur';
const TICKER_URL = `https://www.bitstamp.net/api/v2/ticker/${CURRENCY_PAIR}/`;
const BUY_LIMIT_ORDER_URL = `https://www.bitstamp.net/api/v2/buy/${CURRENCY_PAIR}/`;

if (!AWS.config.region) {
    AWS.config.update({ region: 'eu-central-1' });
}

const ENCRYPTED_BITSTAMP_CUSTOMER_ID = process.env.BITSTAMP_CUSTOMER_ID;
const ENCRYPTED_BITSTAMP_KEY = process.env.BITSTAMP_KEY;
const ENCRYPTED_BITSTAMP_SECRET = process.env.BITSTAMP_SECRET;
const VALUE = Number(process.env.VALUE);
const MINIMUM_VALUE = 5;

if (isNaN(VALUE) || VALUE < MINIMUM_VALUE) {
    throw new Error(`Invalid value [${value}], must be a number not less than [${MINIMUM_VALUE}].`);
}

const kms = new AWS.KMS();

const decrypt = function(encrypted) {
    return kms
        .decrypt({ CiphertextBlob: new Buffer(encrypted, 'base64') })
        .promise()
        .then(data => data.Plaintext.toString('ascii'))
};

let BITSTAMP_PROPERTIES;

const decryptBitstampProperties = function() {
    return Promise.all([
        decrypt(ENCRYPTED_BITSTAMP_CUSTOMER_ID),
        decrypt(ENCRYPTED_BITSTAMP_KEY),
        decrypt(ENCRYPTED_BITSTAMP_SECRET)
    ]).then(([customerId, key, secret]) =>
        BITSTAMP_PROPERTIES = { customerId, key, secret }
    );
})};

const doWithBitstampProperties = function() {
    return BITSTAMP_PROPERTIES
        ? Promise.resolve(BITSTAMP_PROPERTIES)
        : decryptBitstampProperties();
};

const createAuthData = function(customerId, key, secret) {
    const nonce = Date.now();
    const message = nonce + customerId + key;
    const signature = crypto
    .createHmac('sha256', new Buffer(secret, 'utf8'))
    .update(message)
    .digest('hex')
    .toUpperCase();

    return { key, signature, nonce };
};

const getAskPrice = function() {
  return got(TICKET_URL).then(response => {
    const result = JSON.parse(result);
    const ask = Number(result.ask);

    if (!isNaN(ask)) {
      return ask;
    }

    throw new Error(`Could not parse the ask price from [${BTCEUR_TICKER_URL}] response:\n${response.body}`);
  });
});

const buyLimitOrder = function(customerId, key, secret, value, askPrice) {
    const amount = value / askPrice;
    const roundedAmount = Math.ceil(amount * Math.pow(10,5)) / Math.pow(10,5);

    const order = {
        ...createAuthData(customerId, key, secret),
        amount: roundedAmount,
        price: askPrice
    };

    console.log(`Buying ${roundedAmount} of BTC for price ${askPrice} for total value of ${roundedAmount * askPrice}.`);

    return got.post(BUY_LIMIT_ORDER_URL, {
        body: order,
        form: true
    }).then(response => {
        return JSON.parse(response.body);
    });
};

const getAskPriceAndBuyLimitOrder = function(customerId, key, secret, value) {
    return getAskPrice()
        .then(askPrice => buyLimitOrder(customerId, key, secret, value, askPrice));
};

exports.handler = (event, context, callback) => {
    doWithBitstampProperties()
        .then(bitstampProperties => 
            getAskPriceAndBuyLimitOrder(bitstampProperties.customerId, bitstampProperties.key, bitstampProperties.secret, VALUE)
                .then(result => callback(null, result))
                .catch(error => callback(error)))
        .catch(error => callback(error));
};

A few other potential changes...

  • Preferring const over let to denote a variable can't be reassigned (but still mutable)

  • Date.now() can be used instead of (new Date()).getTime()

  • Returning early to prevent else blocks

  • Moving BITSTAMP_PROPERTIES declaration closer to where it's assigned

  • Using the spread operator to prevent mutating orders (Note: this is currently a stage 3 draft at time of writing)

  • Using template literals for more succinct strings

  • Using array matching instead of indices to for customerId, key and secret

  • Using node-fetch instead of got to be consistent with the browser's Fetch API. JSON requests would be of the form fetch(...).then(res => res.json()).then(data => ...) instead of calling JSON.parse

\$\endgroup\$
1
  • \$\begingroup\$ Glad to hear it's helped, @JavierGarcíaManzano! \$\endgroup\$
    – Craig Ayre
    Commented Sep 13, 2017 at 8:19
1
\$\begingroup\$

This is not perfect and I haven't test it, but you can perfectly use async/await in here.

I think there are too many promise chains in your code and it's hard to understand how it works.

As I said, I haven't tried it, maybe there is something interesting for you.

Added

  • try/catch
  • comments
  • async/await
  • shorthand functions

// packages
const AWS = require('aws-sdk');
const crypto = require('crypto');
const got = require('got');

// environment variables
const ENCRYPTED_BITSTAMP_CUSTOMER_ID = process.env['BITSTAMP_CUSTOMER_ID'];
const ENCRYPTED_BITSTAMP_KEY = process.env['BITSTAMP_KEY'];
const ENCRYPTED_BITSTAMP_SECRET = process.env['BITSTAMP_SECRET'];
const VALUE = Number(process.env['VALUE']);

// constants
const CURRENCY_PAIR = 'btceur';
const TICKER_URL = 'https://www.bitstamp.net/api/v2/ticker/' + CURRENCY_PAIR + '/';
const BUY_LIMIT_ORDER_URL = 'https://www.bitstamp.net/api/v2/buy/' + CURRENCY_PAIR + '/';
const MINIMUM_VALUE = 5;

let BITSTAMP_PROPERTIES;

if (!AWS.config.region) {
  AWS
    .config
    .update({region: 'eu-central-1'});
}

if (isNaN(VALUE) || VALUE < MINIMUM_VALUE) {
  throw new Error("Invalid value [" + VALUE + "], must be a number not less than [" + MINIMUM_VALUE + "].");
}

const kms = new AWS.KMS();

/**
 * decrypt
 * @param {*} encrypted
 */
const decrypt = (encrypted) => {
  return new Promise((resolve, reject) => {
    kms
      .decrypt({
      CiphertextBlob: new Buffer(encrypted, 'base64')
    })
      .promise()
      .then(data => resolve(data.Plaintext.toString('ascii')))
      .catch(reject);
  });
};

/**
 * decryptBitstampProperties
 */
const decryptBitstampProperties = async () => {
  try {
    const decryptCustomerId = await decrypt(ENCRYPTED_BITSTAMP_CUSTOMER_ID);
    const decryptBitstampKey = await decrypt(ENCRYPTED_BITSTAMP_KEY);
    const decryptBitstampSecret = await decrypt(ENCRYPTED_BITSTAMP_SECRET);

    return new Promise((resolve, reject) => {
      Promise
        .all([decryptCustomerId, decryptBitstampKey, decryptBitstampSecret])
        .then(values => {
          BITSTAMP_PROPERTIES = {
            customerId: values[0],
            key: values[1],
            secret: values[2]
          };
          resolve(BITSTAMP_PROPERTIES);
        })
        .catch(reject);
    });
  } catch (e) {
    throw new Error(e);
  }
};

/**
 * doWithBitstampProperties
 */
const doWithBitstampProperties = async () => {
  try {
    const result = await decryptBitstampProperties();
    return new Promise((resolve) => {
      resolve(BITSTAMP_PROPERTIES || result);
    });
  } catch (e) {
    throw new Error(e);
  }
};

/**
 * createAuthData
 * @param {*} customerId
 * @param {*} key
 * @param {*} secret
 */
const createAuthData = (customerId, key, secret) => {
  const nonce = new Date().getTime();
  const message = nonce + customerId + key;
  const signature = crypto
    .createHmac('sha256', new Buffer(secret, 'utf8'))
    .update(message)
    .digest('hex')
    .toUpperCase();

  return {
    key, 
    signature,
    nonce
  };
};

/**
 * getAskPrice
 */
const getAskPrice = async () => {
  try {
    const response = await got(TICKER_URL);

    return new Promise((resolve) => {
      const result = JSON.parse(response.body);
      const ask = Number(result.ask);

      if (!isNaN(ask)) {
        resolve(ask);
      } else {
        throw new Error("Could not parse the ask price from [" + BTCEUR_TICKER_URL + "] response:\n" + response.body);
      }
    })
  } catch (e) {
    throw new Error(e);
  }
};

/**
 * buyLimitOrder
 * @param {*} customerId
 * @param {*} key
 * @param {*} secret
 * @param {*} value
 * @param {*} askPrice
 */
const buyLimitOrder = async (customerId, key, secret, value, askPrice) => {
  const amount = value / askPrice;
  const roundedAmount = Math.ceil(amount * Math.pow(10, 5)) / Math.pow(10, 5);
  const order = createAuthData(customerId, key, secret);

  order.amount = roundedAmount;
  order.price = askPrice;

  try {
    console.log("Buying " + roundedAmount + " of BTC for price " + askPrice + " for total value of " + (roundedAmount * askPrice) + ".");
    const response = await got.post(BUY_LIMIT_ORDER_URL, {
      body: order,
      form: true
    });

    return new Promise((resolve) => {
      resolve(JSON.parse(response.body));
    });
  } catch (e) {
    throw new Error(e);
  }
};

/**
 * getAskPriceAndBuyLimitOrder
 * @param {*} customerId
 * @param {*} key
 * @param {*} secret
 * @param {*} value
 */
const getAskPriceAndBuyLimitOrder = async (customerId, key, secret, value) => {
  try {
    const askPrice = await getAskPrice();
    const result = await buyLimitOrder(customerId, key, secret, value, askPrice);

    return new Promise((resolve) => {
      resolve(result);
    });
  } catch (e) {
    throw new Error(e);
  }
}

/**
 * exports.handler
 */
exports.handler = async (callback) => {
  try {
    const bitstampProperties = await doWithBitstampProperties();
    const result = await getAskPriceAndBuyLimitOrder(bitstampProperties.customerId, bitstampProperties.key, bitstampProperties.secret, VALUE);

    callback(null, result);
  } catch (e) {
    callback(e);
  }
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.