fix: 馃悰 fix bug where only the default signer was used #831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
in Procedure `prepare` method, the passed in, non modified `context` was used, when `ctx` was supposed to be used instead
|
alternatively, we could change
overriding the passed in context, so it would be harder to use by mistake. It should be safe since the given variable is a pointer and should leave the caller context unchanged as |
|
Since the REST API lets the SDK initialize first and then set account it would always throw Which is probably better behavior than only using the same signer no matter what its told to use. |
|
Kudos, SonarCloud Quality Gate passed!聽 聽 |
monitz87
approved these changes
Aug 25, 2022
It was only in alpha branch besides the eliminate TQ PR. Usually with sandbox I always init the signer before the SDK so I probably wouldn't have caught it either. It also looks right when reviewing it. |
|
馃帀 This PR is included in version 15.1.0-alpha.2 馃帀 The release is available on: Your semantic-release bot 馃摝馃殌 |
polymath-eric
added a commit
that referenced
this pull request
Sep 15, 2022
* feat: 馃幐 allow for exempted identities to be removed when a user does not pass exempted identities that are exempt when calling setTransferRestrictions the procedure will unset those permissions BREAKING CHANGE: 馃Ж non passed exempted identities are removed when calling setTransferRestriction * refactor: 馃挕 fix code smells * test: 馃拲 add missing coverage * test: 馃拲 clean up test assertions for restrictions * test: 馃拲 add case for restriction type to stat type conversion * refactor: 馃挕 address pr comments * docs: 鉁忥笍 add comments to transformInput for set restrictions * refactor: 馃挕 use function for pushing exemption transactions * refactor: 馃挕 adjust exemption record structure * docs: 鉁忥笍 add comments to addExemptionIfNotPresent method * refactor: 馃挕 remove unused default * style: 馃拕 add line break after sizeOf definition * refactor: 馃挕 Make StatType internal as users never need it The stat type is inferred from the TransferRestrictionType they are interacting with. This eliminates the redundant StatisticsOpType and use StatType through out * docs: 鉁忥笍 add docs around StatType * refactor: 馃挕 address PR comments * refactor: 馃挕 use destructre for cleaner syntax * Beta into alpha (#825) * docs: 鉁忥笍 update compatible version and docs url in readme.md * feat: 馃幐 bump supported chain version to 5.0.2 * fix: 馃悰 add warning to use the new npm repository * chore: 馃 update references to use association signing managers * fix: 馃悰 missing readme from npm * fix: 馃悰 update polymathnetwork to association references (#817) * Update README.md Co-authored-by: Prashant Bajpai <34747455+prashantasdeveloper@users.noreply.github.com> * fix: 馃悰 Fix inviteAccount method by updating the permission type (#821) * fix: 馃悰 Fix transfer with memo (#822) use long type name when creating memos. Also replace lodash padEnd with the built in function Co-authored-by: Victor Vicente <VictorVicente@users.noreply.github.com> * Merge pull request #818 from PolymeshAssociation/beta (#824) Co-authored-by: Victor Vicente <VictorVicente@users.noreply.github.com> Co-authored-by: Victor <victor.g.vicente@gmail.com> Co-authored-by: Victor Vicente <VictorVicente@users.noreply.github.com> Co-authored-by: Prashant Bajpai <34747455+prashantasdeveloper@users.noreply.github.com> * Feat/da 148 eliminate transaction queues (#785) * feat: remove Transaction Queues The `TransactionQueue` class has been removed. Most of its methods have been moved over to `PolymeshTransaction` and `PolymeshTransactionBatch`. Methods that returned a `TransactionQueue` (all endpoints that write to the blockchain, such as `sdk.assets.registerTicker`) now return either a `PolymeshTransaction` or `PolymeshTransactionBatch`. This means that all operations in the SDK are now atomic and not prone to race conditions or half-states - The `onProcessedByMiddleware` method has been moved to `PolymeshTransaction` and `PolymeshTransactionBase` - The `Fees` interface now contains a `total` property which is the sum of `free` and `locked` - The `PayingAccountType` enum now includes a `Caller` member, to represent cases where the calling Account has to pay for a transaction鈥檚 fees BREAKING CHANGES: - Remove the `inputArgs` property from `PolymeshTransaction`. Replaced by the `args` property. Arguments for `PolymeshTransaction` and `PolymeshTransactionBatch` are now available at any point in time (previously they could depend on the result of a previous transaction in the queue) - Remove the `isCritical` property from both `PolymeshTransaction` and `PolymeshTransactionBatch`. It no longer makes sense without Transaction Queues - Change the `run` method in `PolymeshTransaction` and `PolymeshTransactionBatch` to work similarly to the `run` method in `TransactionQueue`. The method will check if the caller account has enough balance for fees, the transaction will be run and its status updated. The method returns a Promise that resolves to the result of running the transaction. For example, calling `run` on the transaction returned by `asset.registerTicker` will return a Promise that resolves to a `TickerReservation` entity - Change the argument received by the callback passed to `onStatusChange` to `PolymeshTransactionBase`. If type refinement is required, the `isPolymeshTransaction` and `isPolymeshTransactionBatch` typeguards can be used (they can be imported from `types`) - Change the return type of `getFees` in `PolymeshTransaction` and `PolymeshTransactionBatch` to `PayingAccountFees`, which contains details about the Account that will pay for the fees, its remaining balance (before paying), and the fees themselves - Remove the `getPayingAccount` method from `PolymeshTransaction` and `PolymeshTransactionBatch`. Its result is now contained in `getFees` - Change the `PayingAccount` type to now only contain `allowance: BigNumber` when `type` is `Subsidy`. For `Caller` and `Other`, there is no `allowance` * perf: 鈿★笍 use a single call to fetch latest finalized block * chore: 馃 implement a way of discriminating batch events * feat: 馃幐 Allow passing of `nonce` as part of `ProcedureOptions` * refactor: 馃挕 Remove unnecessary nonce value from txSpec * feat: 馃幐 Add `currentNonce` method to `Account` * chore: 馃 Allow async values for nonce * Update src/types/index.ts * feat: support merging transactions into batches - Add an `sdk.createTransactionBatch` endpoint that takes an array of SDK transactions and returns them all batched in a single transaction. The result of running this batch is an array of the results of each transaction in the same order - Fix documentation to reflect the fact that transaction queues no longer exist - Improve transaction fee calculation algorithm to avoid edge cases where estimated fees were different than the actual fees - Expose `getProtocolFees` from `PolymeshTransaction` and `PolymeshTransactionBatch` - Expose the `GenericPolymeshTransaction` type (returned by most endpoints that create transactions) BREAKING CHANGES: - Rename `getFees` to `getTotalFees` in `PolymeshTransaction` and `PolymeshTransactionBatch` * feat: 馃幐 expose the `MapTxWithData` type * docs: 鉁忥笍 improve rendered documentation to handle code snippets * test: 馃拲 use proper type in assertion * feat: 馃幐 support splitting batches Add a `splitTransactions` method to the `PolymeshTransactionBatch` class that returns all individual transactions in the batch. This method is useful when the caller is being subsidized, since batches do not support subsidies * fix: 馃悰 carry manual fees over when simplifying a batch When a procedure that normally produces batches produced a single transaction, we weren't passing manual fee data over * test: 馃拲 polymathnetwork -> polymeshassociation in import path Co-authored-by: Prashant Bajpai <prashant.bajpai19@gmail.com> Co-authored-by: Victor Vicente <VictorVicente@users.noreply.github.com> Co-authored-by: Prashant Bajpai <34747455+prashantasdeveloper@users.noreply.github.com> Co-authored-by: Eric <ericrichardson@polymath.network> * feat: 馃幐 release elimination of transaction queues (#828) triggers release of latest alpha as the previous merge had older commits skipped be semver bot Feat/da 148 eliminate transaction queues (#785) * feat: remove Transaction Queues The `TransactionQueue` class has been removed. Most of its methods have been moved over to `PolymeshTransaction` and `PolymeshTransactionBatch`. Methods that returned a `TransactionQueue` (all endpoints that write to the blockchain, such as `sdk.assets.registerTicker`) now return either a `PolymeshTransaction` or `PolymeshTransactionBatch`. This means that all operations in the SDK are now atomic and not prone to race conditions or half-states - The `onProcessedByMiddleware` method has been moved to `PolymeshTransaction` and `PolymeshTransactionBase` - The `Fees` interface now contains a `total` property which is the sum of `free` and `locked` - The `PayingAccountType` enum now includes a `Caller` member, to represent cases where the calling Account has to pay for a transaction鈥檚 fees * perf: 鈿★笍 use a single call to fetch latest finalized block * chore: 馃 implement a way of discriminating batch events * feat: 馃幐 Allow passing of `nonce` as part of `ProcedureOptions` * refactor: 馃挕 Remove unnecessary nonce value from txSpec * feat: 馃幐 Add `currentNonce` method to `Account` * chore: 馃 Allow async values for nonce * Update src/types/index.ts * feat: support merging transactions into batches * feat: 馃幐 expose the `MapTxWithData` type * docs: 鉁忥笍 improve rendered documentation to handle code snippets * test: 馃拲 use proper type in assertion * feat: 馃幐 support splitting batches * fix: 馃悰 carry manual fees over when simplifying a batch Add a `splitTransactions` method to the `PolymeshTransactionBatch` class that returns all individual transactions in the batch. This method is useful when the caller is being subsidized, since batches do not support subsidies When a procedure that normally produces batches produced a single transaction, we weren't passing manual fee data over - Add an `sdk.createTransactionBatch` endpoint that takes an array of SDK transactions and returns them all batched in a single transaction. The result of running this batch is an array of the results of each transaction in the same order - Fix documentation to reflect the fact that transaction queues no longer exist - Improve transaction fee calculation algorithm to avoid edge cases where estimated fees were different than the actual fees - Expose `getProtocolFees` from `PolymeshTransaction` and `PolymeshTransactionBatch` - Expose the `GenericPolymeshTransaction` type (returned by most endpoints that create transactions) BREAKING CHANGES: - Remove the `inputArgs` property from `PolymeshTransaction`. Replaced by the `args` property. Arguments for `PolymeshTransaction` and `PolymeshTransactionBatch` are now available at any point in time (previously they could depend on the result of a previous transaction in the queue) - Remove the `isCritical` property from both `PolymeshTransaction` and `PolymeshTransactionBatch`. It no longer makes sense without Transaction Queues - Change the `run` method in `PolymeshTransaction` and `PolymeshTransactionBatch` to work similarly to the `run` method in `TransactionQueue`. The method will check if the caller account has enough balance for fees, the transaction will be run and its status updated. The method returns a Promise that resolves to the result of running the transaction. For example, calling `run` on the transaction returned by `asset.registerTicker` will return a Promise that resolves to a `TickerReservation` entity - Change the argument received by the callback passed to `onStatusChange` to `PolymeshTransactionBase`. If type refinement is required, the `isPolymeshTransaction` and `isPolymeshTransactionBatch` typeguards can be used (they can be imported from `types`) - Change the return type of `getFees` in `PolymeshTransaction` and `PolymeshTransactionBatch` to `PayingAccountFees`, which contains details about the Account that will pay for the fees, its remaining balance (before paying), and the fees themselves - Remove the `getPayingAccount` method from `PolymeshTransaction` and `PolymeshTransactionBatch`. Its result is now contained in `getFees` - Change the `PayingAccount` type to now only contain `allowance: BigNumber` when `type` is `Subsidy`. For `Caller` and `Other`, there is no `allowance` - Rename `getFees` to `getTotalFees` in `PolymeshTransaction` and `PolymeshTransactionBatch` * feat: 馃幐 use a more specific type for onStatusChange in txBase (#829) use GenericPolymeshTransaction<ReturnValue, TransformedReturnValue> in onStatusChange handler defined in PolymeshTransactionBase. This makes it easier to define functions to be passed to it * chore: 馃 improve typing around `assembleBatchTransaction` This reduces the need to use the `tuple` util, hopefully making the code a bit more idiomatic * fix: 馃悰 fix bug where only the default signer was used (#831) in Procedure `prepare` method, the passed in, non modified `context` was used, when `ctx` was supposed to be used instead * fix: 馃悰 improve the typing of `splitTransactions` The original types weren't being properly reflected by the compiler, resulting in an array of only 2 transactions always. Also, when batching transactions that had a transformed return value, the compiler was throwing errors. Since we don't care about the original return value, only the transformed one, using `any` is acceptable * test: 馃拲 add missing coverage in setTransferRestriction also remove unnessesary type assertions in createMock functions * test: 馃拲 fix jest timer in PolymeshTransactionBase * refactor: 馃挕 remove redundant await * style: 馃拕 remove commented out code Co-authored-by: Victor Vicente <VictorVicente@users.noreply.github.com> Co-authored-by: Victor <victor.g.vicente@gmail.com> Co-authored-by: Prashant Bajpai <34747455+prashantasdeveloper@users.noreply.github.com> Co-authored-by: Jerem铆as D铆az <monitz87@gmail.com> Co-authored-by: Prashant Bajpai <prashant.bajpai19@gmail.com>
|
馃帀 This PR is included in version 17.0.0-beta.1 馃帀 The release is available on: Your semantic-release bot 馃摝馃殌 |
|
馃帀 This PR is included in version 17.0.0 馃帀 The release is available on: Your semantic-release bot 馃摝馃殌 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
in Procedure
preparemethod, the passed in, non modifiedcontextwasused, when
ctxwas supposed to be used insteadBreaking Changes
JIRA Link
DA-405
Checklist