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

Too aggressive caching of transaction validation result? #1330

Closed
roosmaa opened this issue Oct 4, 2017 · 5 comments
Closed

Too aggressive caching of transaction validation result? #1330

roosmaa opened this issue Oct 4, 2017 · 5 comments
Assignees

Comments

@roosmaa
Copy link

roosmaa commented Oct 4, 2017

In the following scenario, the transaction validation result caching seems to be too aggressive.

  1. Create account with 50XLM and a prepared tx that transfers 100XLM
  2. Try to execute the prepared tx, get op_underfunded
  3. Send 100XLM to the account
  4. Try to execute the prepared tx again, but still get op_underfunded eventhough the funds are present

The same caching problem happens when the prepared tx result can yield different results based on the signers of the tx. For instance if one signer doesn't have enough weight and tx fails, that failure is cached and when the signer who does have enough weight tries to submit it, the failure is still returned.

Here's a test case for the op_underfunded:

var BigNumber = require('bignumber.js');
var StellarSdk = require('stellar-sdk');

StellarSdk.Network.useTestNetwork();
var server = new StellarSdk.Server('https://horizon-testnet.stellar.org');

var mainKeypair = StellarSdk.Keypair.fromSecret('SADVYXS74A3NGJJT6UFEX4JUD27F46DCWURFMCXFZN5RESICKQ5GWQXB'),
    newKeypair = StellarSdk.Keypair.random();

console.log('New account:', newKeypair.publicKey(),
      'secret:', newKeypair.secret());

var newAccountSeq;

server.loadAccount(mainKeypair.publicKey())
  .then(printBalances('main'))
  .then(function (mainAccount) {
    console.log('Creating new account');

    var tx = new StellarSdk.TransactionBuilder(mainAccount)
      .addOperation(StellarSdk.Operation.createAccount({
        destination: newKeypair.publicKey(),
        startingBalance: '50',
      }))
      .build();
    tx.sign(mainKeypair);
    return server.submitTransaction(tx);
  })
  .then(function () {
    return server.loadAccount(newKeypair.publicKey());
  })
  .then(printBalances('new'))
  .then(function (newAccount) {
    console.log('Registering payment tx hash');

    newAccountSeq = newAccount.sequenceNumber();
    paymentTx = buildPaymentTx();

    var tx = new StellarSdk.TransactionBuilder(newAccount)
      .addOperation(StellarSdk.Operation.setOptions({
        signer: {
          preAuthTx: paymentTx.hash(),
          weight: 1,
        },
      }))
      .build();
    tx.sign(newKeypair);

    return server.submitTransaction(tx);
  })
  .then(function () {
    console.log('Attempting to send non-existent money');

    var tx = buildPaymentTx();
    return server.submitTransaction(tx)
      .catch(function (error) {
        console.log('(intercepted expected error)');
      });
  })
  .then(function () {
    return server.loadAccount(mainKeypair.publicKey());
  })
  .then(function (mainAccount) {
    console.log('Sending more money to new account');

    // Lock account with one valid presigned tx
    var tx = new StellarSdk.TransactionBuilder(mainAccount)
      .addOperation(StellarSdk.Operation.payment({
        destination: newKeypair.publicKey(),
        asset: StellarSdk.Asset.native(),
        amount: '100',
      }))
      .build();
    tx.sign(mainKeypair);

    return server.submitTransaction(tx);
  })
  .then(function () {
    console.log('Attempting to send money back to main account');

    var tx = buildPaymentTx();
    return server.submitTransaction(tx);
  })
  .then(function (result) {
    console.log('SUCCESS!');
  })
  .catch(function (error) {
    console.log('ERROR:', error);
    console.log('extras.result_codes: ', error.extras.result_codes);
  });

function buildPaymentTx() {
  var seq = new BigNumber(newAccountSeq);
  var account = new StellarSdk.Account(newKeypair.publicKey(), seq.add(1).toString());

  var tx = new StellarSdk.TransactionBuilder(account)
    .addOperation(StellarSdk.Operation.payment({
      destination: mainKeypair.publicKey(),
      asset: StellarSdk.Asset.native(),
      amount: '100',
    }))
    .build();
  return tx;
}

function printBalances(accountName) {
  return function (acc) {
    console.log('Balances for ' + accountName + ' account:');
    acc.balances.forEach(function(balance) {
      console.log('  Type:', balance.asset_type, ', Balance:', balance.balance);
    });
    console.log();

    return acc;
  };
}

How long would one have to wait right now for the cache entry to expire to "workaround" this problem?

@vogel vogel self-assigned this Oct 4, 2017
@vogel
Copy link
Contributor

vogel commented Oct 5, 2017

I think you are trying to sent two transactions with the same sequence number. op_underfunded means that sequence number was consumed for given account and you have to use next one.

Please reopen if using next sequence number didn't work.

@vogel vogel closed this as completed Oct 5, 2017
@roosmaa
Copy link
Author

roosmaa commented Oct 5, 2017

@vogel but that's the thing.. if it's a pre-authorized transaction, you can't change the sequence number.

@vogel
Copy link
Contributor

vogel commented Oct 5, 2017

In that case we need another solution, but it should be discussed on stellar-protocol project.
For example:

  • transaction with sequence-number-range instead of sequence-number
  • transaction with wildcard sequence-number (like 0, which could be executed at any time).

With current protocol you have to ensure that transaction can be executed or sequence number will be consumed and pre-authorized transaction will be useless.

One solution for that is to create account that has all needed funds and only allow predefined set of transaction to be executed on it (with proper setting of thresholds and signers), like:

  • send payment to account XYZ (with no time bounds)
  • merge back with source account (with some time bounds and the same seqnum as previous transaction)

@roosmaa
Copy link
Author

roosmaa commented Oct 5, 2017

@vogel just so that we're on the same page - even if the transaction fails to be applied, the account sequence number is still incremented?

@vogel
Copy link
Contributor

vogel commented Oct 5, 2017

It depends. But for all op_ errors yes, sequence number is incremented.
For tx_ errors I would have to check, but I think sequence number is not incremented for all tx_ erros.

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

No branches or pull requests

2 participants