asch icon indicating copy to clipboard operation
asch copied to clipboard

Contracts which invoke the balances are not acid compliant

Open bassjobsen opened this issue 7 years ago • 1 comments

Version

1.4

Environment

linux

Steps to reproduce

from https://github.com/AschPlatform/cctime/blob/master/contract/cctime.js#L90 the code (in summary) looks like that shown below:

    let balance = app.balances.get(this.trs.senderId, VOTE_CURRENCY)
    app.balances.decrease(this.trs.senderId, VOTE_CURRENCY, amount)
    app.balances.increase(article.authorId, VOTE_CURRENCY, authorReward)

What is expected?

Something like that shown beneath:

    let balance = app.balances.get_for_update(this.trs.senderId, VOTE_CURRENCY)
    // get_for_update should lock the `balance` table
    app.balances.starttranaction()
      app.balances.decrease(this.trs.senderId, VOTE_CURRENCY, amount)
      app.balances.increase(article.authorId, VOTE_CURRENCY, authorReward)
   app.balances.commit()

also see: https://sqlite.org/lang_transaction.html

What is actually happening?

In the current situation you can not ensure that the balance does still contain enough assets when app.balances.decrease() run actually. Also we got some troubles when app.balances.decrease() fails and app.balances.decrease() did not fail or visa versa.

bassjobsen avatar Aug 05 '18 11:08 bassjobsen

In fact the main problem seems to be that one shouldn't not be able to invoke a increase or decrease of the balance at all. A contract should always do a transfer (from the balance of the sender to the balance of the receiver). transfer() should check if the balance of the sender contains enough assets. So i suggest to replace https://github.com/AschPlatform/asch-core/blob/master/src/smartdb/balance-manager.js with the code shown beneath:

function getCurrencyFlag(currency) {
  if (currency === 'XAS') {
    return 1
  } if (currency.indexOf('.') !== -1) {
    // UIA
    return 2
  }
  // gateway currency
  return 3
}

// "protected" functions
increase(address, currency, amount) {
  if (app.util.bignumber(amount).eq(0)) return
  const key = { address, currency }
  let item = this.sdb.get('Balance', key)
  if (item) {
    item.balance = app.util.bignumber(item.balance).plus(amount).toString(10)
    app.sdb.update('Balance', { balance: item.balance }, key)
   } else {
    item = this.sdb.create('Balance', {
      address,
      currency,
      balance: amount,
      flag: getCurrencyFlag(currency),
    })
  }
}

decrease(address, currency, amount) {
  this.increase(address, currency, `-${amount}`)
}  

class BalanceManager {
  constructor(sdb) {
    this.sdb = sdb
  }

  get(address, currency) {
    const item = this.sdb.get('Balance', { address, currency })
    const balance = item ? item.balance : '0'
    return app.util.bignumber(balance)
  }

  increase(address, currency, amount) {
    return 'You can only increase a balance by doing a transfer'
  }

  decrease(address, currency, amount) {
    return 'You can only decrease a balance by doing a transfer'
  }

  transfer(currency, amount, from, to) {
    app.sdb.beginTrans()
    if(this.get(from, currency) < amount) return 'not enough' + currency + 'to transfer'
    decrease(from, currency, amount)
    increase(to, currency, amount)
    app.sdb.commit() 
  }
}

module.exports = BalanceManager

Notice that the above should not only fix the current issue, but also https://github.com/AschPlatform/asch/issues/229

bassjobsen avatar Aug 05 '18 12:08 bassjobsen