-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add FeeBumpTransaction class #328
Conversation
if I have a xdr envelope string and I want to parse it into a if I call It seems like we want a Is there a way to limit access to the We should compare the tradeoffs between this approach vs just having a single |
Good point! I didn't add that yet. I think we can add it in the builder so it does the right handling depending on the envelope type.
I don't think there is an out the box way to achieve this, as long as people can get a hold on the class they can create new instances. I'm actually okay by allowing people to use this directly, however, we should add a helper to the builder so people can do |
@tamirms also added |
*/ | ||
export class TransactionBase { | ||
constructor(tx, signatures, fee, networkPassphrase) { | ||
if (typeof networkPassphrase !== 'string') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we were suppose to make this change on the previous major bump but we forgot, not this is a good time.
src/transaction.js
Outdated
import { Memo } from './memo'; | ||
import { Keypair } from './keypair'; | ||
import { TransactionBase } from './transaction_base'; | ||
|
||
/** | ||
* Use {@link TransactionBuilder} to build a transaction object, unless you have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is worth adding a comment that we generally discourage creating Transaction
instances manually. Instead people should be using builder.fromXdr()
to construct Transaction
instances. The same comment applies to FeeBumpTransaction
as well.
If this were implemented in typescript I think we'd make Transaction
and FeeBumpTransaction
interfaces which are exported but the implementations would not be exported and only be local to the builder code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* Handle transaction envelope with MuxedAccount in sourceAccount. * Add FeeBumpTransaction. * Add more test for FeeBumpTransaction. * Improve docs. * Add TransactionBase and extend from it. * Use named import and inherit documentation. * Inherit TransactionBase in Transaction. * Document readonly attrs. * Document properties on fee bump tx. * Show M accounts. * Return a copy of tx and signatures when building toEnvelope. * Add helper function fromXDR. * Update types. * Update XDR. * Handle muxed accoutns in fee bump transactions. * Add link to fromXDR in Transaction and FeeBumpTransaction docs. * Extend fromXDR to take a xdr object or a string.
This version brings protocol 13 support with backwards compatibility support for protocol 12. ### Add - Add `TransactionBuilder.buildFeeBumpTransaction` which makes it easy to create `FeeBumpTransaction` ([#321](#321)). - Adds a feature flag which allow consumers of this library to create V1 (protocol 13) transactions using the `TransactionBuilder` ([#321](#321)). - Add support for [CAP0027](https://github.com/stellar/stellar-protocol/blob/master/core/cap-0027.md): First-class multiplexed accounts ([#325](#325)). - Add `Keypair.xdrMuxedAccount` which creates a new `xdr.MuxedAccount`([#325](#325)). - Add `FeeBumpTransaction` which makes it easy to work with fee bump transactions ([#328](#328)). - Add `TransactionBuilder.fromXDR` which receives an xdr envelope and return a `Transaction` or `FeeBumpTransaction` ([#328](#328)). ### Update - Update XDR definitions with protocol 13 ([#317](#317)). - Extend `Transaction` to work with `TransactionV1Envelope` and `TransactionV0Envelope` ([#317](#317)). - Add backward compatibility support for [CAP0018](https://github.com/stellar/stellar-protocol/blob/f01c9354aaab1e8ca97a25cf888829749cadf36a/core/cap-0018.md) ([#317](#317)). - Update operations builder to support multiplexed accounts ([#337](#337)). This allows you to specify an `M` account as the destination or source: ``` var destination = 'MAAAAAAAAAAAAAB7BQ2L7E5NBWMXDUCMZSIPOBKRDSBYVLMXGSSKF6YNPIB7Y77ITLVL6'; var amount = '1000.0000000'; var asset = new StellarBase.Asset( 'USDUSD', 'GDGU5OAPHNPU5UCLE5RDJHG7PXZFQYWKCFOEXSXNMR6KRQRI5T6XXCD7' ); var source = 'MAAAAAAAAAAAAAB7BQ2L7E5NBWMXDUCMZSIPOBKRDSBYVLMXGSSKF6YNPIB7Y77ITLVL6'; StellarBase.Operation.payment({ destination, asset, amount, source }); ``` **To use multiplexed accounts you need an instance of Stellar running on Protocol 13 or higher** ### Breaking changes - `Transaction.toEnvelope()` returns a protocol 13 `xdr.TransactionEnvelope` which is an `xdr.Union` ([#317](#317)). If you have code that looks like this `transaction.toEnvelope().tx` you have two options: - You can grab the value wrapped by the union, calling `value()` like `transaction.toEnvelope().value().tx`. - You can check which is the discriminant by using `switch()` and then call `v0()`, `v1()`, or `feeBump()`. - The return value from `Transaction.fee` changed from `number` to `string`. This brings support for `Int64` values ([#321](#321)). - The const `BASE_FEE` changed from `number` to `string` ([#321](#321)). - The option `fee` passed to `new TransactionBuilder({fee: ..})` changed from `number` to `string` ([#321](#321)). - The following fields, which were previously an `xdr.AccountID` are now a `xdr.MuxedAccount` ([#325](#325)): - `PaymentOp.destination` - `PathPaymentStrictReceiveOp.destination` - `PathPaymentStrictSendOp.destination` - `Operation.sourceAccount` - `Operation.destination` (for `ACCOUNT_MERGE`) - `Transaction.sourceAccount` - `FeeBumpTransaction.feeSource` You can get the string representation by calling `StrKey.encodeMuxedAccount` which will return a `G..` or `M..` account. - Remove the following deprecated functions ([#331](#331)): - `Operation.manageOffer` - `Operation.createPassiveOffer` - `Operation.pathPayment` - `Keypair.fromBase58Seed` - Remove the `Network` class ([#331](#331)). - Remove `vendor/base58.js` ([#331](#331)).
What
Add the class
FeeBumpTransaction
to represent fee bump transactions.Why
This pull request updates some of the code introduced in #321, creating a different class to represent fee bump transaction.
While having a single class could work, long term, having different classes to separate both type of transactions is less error prone and makes the interface on fee bump transactions more explicit.
When you create a new
FeeBumpTransaction
you can reach the inner transaction via.innerTransaction
, this returns an instance ofTransaction
which gives access to all the properties on the inner transaction, like the original fee, sourceAccount, operations, and memo.