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

Transaction updates #221

Merged
merged 4 commits into from
Dec 10, 2014
Merged

Transaction updates #221

merged 4 commits into from
Dec 10, 2014

Conversation

wltsmrz
Copy link
Contributor

@wltsmrz wltsmrz commented Dec 8, 2014

No description provided.

// We aren't clever enough to eschew preventative measures so we keep an array
// of all submitted transactionIDs (which can change due to load_factor
// effecting the Fee amount). This should be populated with a transactionID
// any time it goes on the network
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep comments for some of these properties. They might not be clear to other people browsing the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Canonical signing setting defaults to the Remote's configuration should be obvious and so is this.submittedIDs. The comments in this case don't really provide more information than the code itself.

@geertweening
Copy link
Contributor

LGTM, please rebase before merging

- Deprecate 'save' event
- Add TransactionQueue.getMinLedger(), use this as ledger_index_min
  in account_tx request on reconnect
- tx.sign() no longer accepts a callback
- Add various setters and jsdoc to transaction.js
- Normalize setters, e.g. sourceTag() and destinationTag()
- Minor optimization in call to tx.hash() in TransactionManager prior
  to submit; allow `serialized` argument to tx.hash() such that the
  transaction is not serialized twice
Do not finalize tef or tel-class errors until LastLedgerSequence is
exceeded. Transactions that fail in this way will now be aborted
with a tej-class error. Errors like tefALREADY and tefMAX_LEDGER
should now be opaque to the user and have no consequence in
determining a final state.
geertweening added a commit that referenced this pull request Dec 10, 2014
@geertweening geertweening merged commit 0835de9 into develop Dec 10, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling 1a892d5 on transaction-updates into 634e811 on develop.

@wltsmrz wltsmrz deleted the transaction-updates branch December 13, 2014 02:35
ledhed2222 pushed a commit that referenced this pull request Nov 8, 2021
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

Successfully merging this pull request may close these issues.

3 participants