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

Updating account signers in a transaction doesn't take effect for following operations #1329

Closed
roosmaa opened this issue Oct 4, 2017 · 2 comments
Assignees

Comments

@roosmaa
Copy link

roosmaa commented Oct 4, 2017

So I had this "good" idea, of preparing some transactions ahead of time, that add a signer for the duration of that transaction so that only the person with access to the private key could apply the prepared transaction.

var tx = new StellarSdk.TransactionBuilder(new StellarSdk.Account(accountKeypair.publicKey(), '1234'))
  .addOperation(StellarSdk.Operation.setOptions({
    // The idea is that the transaction hash (which is already a signer)
    // contributes 1 to the weight and then the signingKeypair added
    // below controbutes another 1, totalling 2.
    lowThreshold: 2,
    medThreshold: 2,
    highThreshold: 2,
    signer: {
      ed25519PublicKey: signingKeypair.publicKey(),
      weight: 1,
    },
  }))
  // At this point the thresholds seem to get updated,
  // but the signers list not as the following 3 operations
  // return op_bad_auth
  .addOperation(StellarSdk.Operation.payment({
    destination: recipientKeypair.publicKey(),
    asset: StellarSdk.Asset.native(),
    amount: '10',
  }))
  .addOperation(StellarSdk.Operation.setOptions({
    // Reset account back to what it was before this transaction
    lowThreshold: 1,
    medThreshold: 1,
    highThreshold: 1,
    signer: {
      ed25519PublicKey: signingKeypair.publicKey(),
      weight: 0,
    },
  }))
  .build();

tx.sign(signingKeypair);

The prepared transaction definition above would've had it's hash appended to the accounts signers list before. The signing with the signingKeypair and submitting to the network would happen after that. But sadly op_bad_auth would be returned for all operations but the first one.

@roosmaa
Copy link
Author

roosmaa commented Oct 4, 2017

Tried to produce a dedicated testcase, ended up getting tx_bad_auth_extra error instead of op_bad_auth. Anyway, here's the testcase:

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('SBJ3CIHTAISHL3YXE2IKDQ3QGVMUDQLRJ7D2DYCCGKBAZMWFUJNNWQJE'),
    newKeypair = StellarSdk.Keypair.random(),
    signerKeypair = StellarSdk.Keypair.random();

console.log('New account:', newKeypair.publicKey(),
      'secret:', newKeypair.secret());
console.log('Signer:', signerKeypair.publicKey(),
      'secret:', signerKeypair.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: '100',
      }))
      .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('Sending money back to main account');

    var tx = buildPaymentTx();
    // Should work with either this:
    paymentTx.sign(signerKeypair);
    // ..or this:
    // paymentTx.sign(newKeypair);
    return server.submitTransaction(paymentTx);
  })
  .then(function () {
    return server.loadAccount(newKeypair.publicKey());
  })
  .then(printBalances('new'))
  .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.setOptions({
      lowThreshold: 2,
      medThreshold: 2,
      highThreshold: 2,
      signer: {
        ed25519PublicKey: signerKeypair.publicKey(),
        weight: 1,
      },
    }))
    .addOperation(StellarSdk.Operation.payment({
      destination: mainKeypair.publicKey(),
      asset: StellarSdk.Asset.native(),
      amount: '10',
    }))
    .addOperation(StellarSdk.Operation.setOptions({
      lowThreshold: 1,
      medThreshold: 1,
      highThreshold: 1,
      signer: {
        ed25519PublicKey: signerKeypair.publicKey(),
        weight: 0,
      },
    }))
    .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;
  };
}

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

vogel commented Oct 5, 2017

Hi.

Validating transaction signatures is done before any operation is applied. So all the verification takes place before needed weight is increased, therefore only the source account signature is needed and presence of "signingKeypair" signature is reported with tx_bad_auth_extra error.

If you want this behavior to be changed, please post an issue to stellar-protocol project, but I think it would be better to figure out another way of doing what you need.

For example: it may be best to create a separate account which only purpose is to execute these transactions that need "signingKeypair" signature. You could fund it with enough money and set signers appropriately.

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