Throw error on duplicate withdrawal detected in federation wallet#1126
Merged
quantumagi merged 9 commits intostratisproject:release/1.4.0.7from Mar 6, 2023
Merged
Conversation
zeptin
approved these changes
Mar 6, 2023
fr0stnk
pushed a commit
to fr0stnk/StratisFullNode
that referenced
this pull request
Mar 12, 2023
…ratisproject#1126) * Limit how often the wallet is saved * Tweak * Force wallet save in stop * Throw error on duplicate withdrawal detected in federation wallet * Stop application * Fix * Just display an error for now * Can only explicitly remove transient transactions
StratisIain
added a commit
that referenced
this pull request
May 31, 2023
* Removed unused external lib folder * revert back external lib * upgrade to .NET 6 * Fixed Issue of POA IntegrationTests and Patricia Test * Shorten long test name * Migrate changes from PR #957 * Set official release version * upgrade to .net 6 * Fix uni tests * Remove specific runtime version * Bump version to 1.3.3.0 * Bump all versions * Bump collateral and federated peg versions * Bump NBitcoin version * Fix duplicate publish file output issue * Fix duplicate publish file output issue (#1027) * GetEntireState and potential block skipping fix (#1037) * Bump all versions to 1.3.3.0 * Create initial classes * Add SignalR event * WIP * Done * Payload Fixes * Update MultiSigStateMonitorBehavior.cs * Update MultiSigStateMonitorBehavior.cs * Merge pull request #1040 from zeptin/transactionsigners-20220815 [API] Add gettransactionsigners endpoint * Update FederationGatewayController.cs * Add switch disable monitoring * Update MultiSigStateMonitorBehavior.cs * Add net6 version suffix * Remove duplicate package reference * Merge pull request #1045 from quantumagi/fixhistrecvquery Fix history receive query * Bump all versions to 1.3.3.0 * Fix decompiler Address references * Temporary pre-merge * Merge pull request #1048 from quantumagi/singlenodestartdate Delegate node StartTime to NodeStats only * Create WatchedSrc721Contracts setting instance * Add already include check. * Add index to filter * Update launch script to enable watcherc721 contract addreses * Add logging and timeout * Bump build version * Update Mint Log for NFTs * Fix console NFT log * Small console log change * Extend receipt searcher to support multiple contracts * Update NFT indexer to batch receipt searches * Use extension method for bloom filter testing * Allow wildcard contract address receipt searches * Check contract type via local call * Track last updated block centrally * Changes per review * Refactor to use async loop * Fix cherry picking * Merge pull request #1055 from zeptin/receiptsearch-20220910 NFT indexing contract address fix * Update NFTIndexer logs * Update NFTTransferIndexer.cs * Remove test line * Expedited updates from 1.4 (#1058) * Bump build revision * Remove UnityApi from StraxD * Bump all versions to 1.4.0.0 * Update Dockerfiles for .NET 6 * Fix Dockerfile * Add "getverboseaddressesbalances2" with POST API (#1061) (#1063) * Add getverboseaddressesbalances2 with POST API * Remove arguments * Update tests * Renaming based on feedback * Fix comment Co-authored-by: quantumagi <someguy.fromafrica@gmail.com> * Use dummy address for local calls by default (#1067) * Bump build version for testing * Update Token Contract addresses for ETH Sepolia * Bump build version for testing * Fix TSZ1 token contract address * Merge pull request #973 from SatyaKarki/release/1.4.0.0 Add TransactionAddedToMemoryPool SignalR Event * Merge pull request #977 from SatyaKarki/release/1.4.0.0 add memorySizePool property for SignalR event * Merge pull request #989 from SatyaKarki/release/1.4.0.0 Create and Publish MiningStatistics SignalR event * Merge pull request #1032 from SatyaKarki/release/1.4.0.0 Add Consensus manager SignalR event * Merge pull request #1038 from SatyaKarki/release/1.4.0.0 Add header height in consensus manager SignalR event * Merge pull request #1048 from quantumagi/singlenodestartdate Delegate node StartTime to NodeStats only * Merge pull request #1047 from SatyaKarki/release/1.4.0.0 Add node start time * Merge pull request #1060 from SatyaKarki/release/1.4.0.0 Add a new peer connection signalr event and send list of connections in signalr message * Fix build post merge of Satya's SignalR changes * Small bit of cleanup post merge * Bump build version to 1.4.0.4 for testing * Merge pull request #942 from SatyaKarki/release/1.4.0.0 enable SignalR in CirrusMinerD * Publish federation wallet changes via signalr * Bump build version to 1.4.0.5 * Merge pull request #1084 from zeptin/dumpprivkey-20221031 [RPC] Add dumpprivkey RPC command * Publish Address Indexer Tip * Bump build version * WIP * Fix ref * Fix status_information_is_returned test * Bump build version * Merge legacy PRs into release 1.4.0.7 (#1112) * Bump all versions to 1.3.4.0 * Bump all versions to 1.4.0.0 * Set version to RC * Always publish mining statistics * Add activation height for multisig fee sorting * Fix GateWayPairStarts pair * Update release 1.4 activation height * Bump build version to 1.4.0.1 * Fix multisig fee sort * Bump build revision to 1.4.0.2 * Set release 1.4 activation height * Set official release version * Merge legacy PRs into release 1.4.0.0 (#1111) * Add dumpprivkey RPC command * Revert superfluous change * Add additional sanity test * Fix test * [RPC] Add createrawtransaction RPC method (#1100) * Add createrawtransaction RPC method * Better handling of Sequence field, and safer defaults * Fix case where fundrawtransaction is called for the watch-only account * [RPC] Fix createrawtransaction zero-input handling (#1102) * Add createrawtransaction RPC method * Better handling of Sequence field, and safer defaults * Fix case where fundrawtransaction is called for the watch-only account * Add protocol version overloads for CreateTransaction * Check that createrawtransaction RPC client works for SFN/bitcoind * Modify createrawtransaction RPC to return string response * Fix RPC middleware to correctly handle non-JSON RPC responses * Remove accidental duplication * Fix test * Revert middleware change and add additional test Co-authored-by: Kevin Loubser <zeptin@gmail.com> Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com> Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com> Co-authored-by: Kevin Loubser <zeptin@gmail.com> * Merge legacy PRs into release 1.4.0.7 - part 2 of 2 (#1113) * Merge legacy PRs into release 1.4.0.7 - part 2 of 2 * Override base CreateTransaction * Fix VerboseAddressBalancesData (#1115) * Bump versions to 1.4.0.7 (#1114) * Bump versions to 1.4.0.7 * Revert unintended change * Fix CI issues (#1120) * Improve missing input logging with debug log (#1121) * Fix CI errors on 1.4.0.7 (#1122) * Fix FailsFV testcase * Don't dispose already disposed runners * Improve error logging in FullNodeFeatureExecutor * Simplify AsyncLock * Fix * Increase timeout * Disable failing Ubuntu and OSX test case temporarily * Stabilize test * Improve test-case * Implement bounded LRU map for peer known inv hashes * Remove spurious 'using' * Make WalletSyncManager more robust * Add fix from upstream NBitcoin * Add missing payload attribute * Don't process deposits prior to already processed deposits (#1124) * Don't process deposits prior to already processed deposits * Fix MaturedBlocksSyncManagerTests * Update src/Stratis.Features.FederatedPeg/TargetChain/MaturedBlocksSyncManager.cs Co-authored-by: zeptin <zeptin@gmail.com> * Fix BlocksAreRequestedIfThereIsSomethingToRequestAsync --------- Co-authored-by: zeptin <zeptin@gmail.com> * Limit how often the wallet is saved (#1125) * Limit how often the wallet is saved * Tweak * Force wallet save in stop * Throw error on duplicate withdrawal detected in federation wallet (#1126) * Limit how often the wallet is saved * Tweak * Force wallet save in stop * Throw error on duplicate withdrawal detected in federation wallet * Stop application * Fix * Just display an error for now * Can only explicitly remove transient transactions * Fix conversion in benchmarks * Add list shuffle helper method * Add P2WSH handling to wallet database * Fix signalr config * Add check for empty list * Extend the PR 624 changes to reward distribution (#1128) * Add maturity height to logging (#1127) * Add maturity height to logging * Restore removed code section * Send potentially suspended tx's to CCTS * Fix * Logging and code improvements * Fix null and enforce sync * Resolve some error scenarios * Cleanup * Remove redundant code (#1129) * Prevent non-ascii characters being omitted from console (#1130) * Use deposit blocktime for conversions (#1142) * Use deposit blocktime for conversions * Fix build errors in tests * Update Launch Scripts * Additional endpoint functionality (#1146) --------- Co-authored-by: Satya Karki <spacegrlsatya@gmail.com> Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com> Co-authored-by: Nikita Koifman <nik7966@icloud.com> Co-authored-by: noescape0 <NoEscape0@ya.ru> Co-authored-by: quantumagi <someguy.fromafrica@gmail.com> Co-authored-by: Iain McCain <iain.mccain@stratisplatform.com> Co-authored-by: Nikita Koifman <nikita.koifman@stratisplatform.com>
StratisIain
added a commit
that referenced
this pull request
May 31, 2023
* Removed unused external lib folder * revert back external lib * upgrade to .NET 6 * Fixed Issue of POA IntegrationTests and Patricia Test * Shorten long test name * Migrate changes from PR #957 * Set official release version * upgrade to .net 6 * Fix uni tests * Remove specific runtime version * Bump version to 1.3.3.0 * Bump all versions * Bump collateral and federated peg versions * Bump NBitcoin version * Fix duplicate publish file output issue * Fix duplicate publish file output issue (#1027) * GetEntireState and potential block skipping fix (#1037) * Bump all versions to 1.3.3.0 * Create initial classes * Add SignalR event * WIP * Done * Payload Fixes * Update MultiSigStateMonitorBehavior.cs * Update MultiSigStateMonitorBehavior.cs * Merge pull request #1040 from zeptin/transactionsigners-20220815 [API] Add gettransactionsigners endpoint * Update FederationGatewayController.cs * Add switch disable monitoring * Update MultiSigStateMonitorBehavior.cs * Add net6 version suffix * Remove duplicate package reference * Merge pull request #1045 from quantumagi/fixhistrecvquery Fix history receive query * Bump all versions to 1.3.3.0 * Fix decompiler Address references * Temporary pre-merge * Merge pull request #1048 from quantumagi/singlenodestartdate Delegate node StartTime to NodeStats only * Create WatchedSrc721Contracts setting instance * Add already include check. * Add index to filter * Update launch script to enable watcherc721 contract addreses * Add logging and timeout * Bump build version * Update Mint Log for NFTs * Fix console NFT log * Small console log change * Extend receipt searcher to support multiple contracts * Update NFT indexer to batch receipt searches * Use extension method for bloom filter testing * Allow wildcard contract address receipt searches * Check contract type via local call * Track last updated block centrally * Changes per review * Refactor to use async loop * Fix cherry picking * Merge pull request #1055 from zeptin/receiptsearch-20220910 NFT indexing contract address fix * Update NFTIndexer logs * Update NFTTransferIndexer.cs * Remove test line * Expedited updates from 1.4 (#1058) * Bump build revision * Remove UnityApi from StraxD * Bump all versions to 1.4.0.0 * Update Dockerfiles for .NET 6 * Fix Dockerfile * Add "getverboseaddressesbalances2" with POST API (#1061) (#1063) * Add getverboseaddressesbalances2 with POST API * Remove arguments * Update tests * Renaming based on feedback * Fix comment Co-authored-by: quantumagi <someguy.fromafrica@gmail.com> * Use dummy address for local calls by default (#1067) * Bump build version for testing * Update Token Contract addresses for ETH Sepolia * Bump build version for testing * Fix TSZ1 token contract address * Merge pull request #973 from SatyaKarki/release/1.4.0.0 Add TransactionAddedToMemoryPool SignalR Event * Merge pull request #977 from SatyaKarki/release/1.4.0.0 add memorySizePool property for SignalR event * Merge pull request #989 from SatyaKarki/release/1.4.0.0 Create and Publish MiningStatistics SignalR event * Merge pull request #1032 from SatyaKarki/release/1.4.0.0 Add Consensus manager SignalR event * Merge pull request #1038 from SatyaKarki/release/1.4.0.0 Add header height in consensus manager SignalR event * Merge pull request #1048 from quantumagi/singlenodestartdate Delegate node StartTime to NodeStats only * Merge pull request #1047 from SatyaKarki/release/1.4.0.0 Add node start time * Merge pull request #1060 from SatyaKarki/release/1.4.0.0 Add a new peer connection signalr event and send list of connections in signalr message * Fix build post merge of Satya's SignalR changes * Small bit of cleanup post merge * Bump build version to 1.4.0.4 for testing * Merge pull request #942 from SatyaKarki/release/1.4.0.0 enable SignalR in CirrusMinerD * Publish federation wallet changes via signalr * Bump build version to 1.4.0.5 * Merge pull request #1084 from zeptin/dumpprivkey-20221031 [RPC] Add dumpprivkey RPC command * Publish Address Indexer Tip * Bump build version * WIP * Fix ref * Fix status_information_is_returned test * Bump build version * Merge legacy PRs into release 1.4.0.7 (#1112) * Bump all versions to 1.3.4.0 * Bump all versions to 1.4.0.0 * Set version to RC * Always publish mining statistics * Add activation height for multisig fee sorting * Fix GateWayPairStarts pair * Update release 1.4 activation height * Bump build version to 1.4.0.1 * Fix multisig fee sort * Bump build revision to 1.4.0.2 * Set release 1.4 activation height * Set official release version * Merge legacy PRs into release 1.4.0.0 (#1111) * Add dumpprivkey RPC command * Revert superfluous change * Add additional sanity test * Fix test * [RPC] Add createrawtransaction RPC method (#1100) * Add createrawtransaction RPC method * Better handling of Sequence field, and safer defaults * Fix case where fundrawtransaction is called for the watch-only account * [RPC] Fix createrawtransaction zero-input handling (#1102) * Add createrawtransaction RPC method * Better handling of Sequence field, and safer defaults * Fix case where fundrawtransaction is called for the watch-only account * Add protocol version overloads for CreateTransaction * Check that createrawtransaction RPC client works for SFN/bitcoind * Modify createrawtransaction RPC to return string response * Fix RPC middleware to correctly handle non-JSON RPC responses * Remove accidental duplication * Fix test * Revert middleware change and add additional test Co-authored-by: Kevin Loubser <zeptin@gmail.com> Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com> Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com> Co-authored-by: Kevin Loubser <zeptin@gmail.com> * Merge legacy PRs into release 1.4.0.7 - part 2 of 2 (#1113) * Merge legacy PRs into release 1.4.0.7 - part 2 of 2 * Override base CreateTransaction * Fix VerboseAddressBalancesData (#1115) * Bump versions to 1.4.0.7 (#1114) * Bump versions to 1.4.0.7 * Revert unintended change * Fix CI issues (#1120) * Improve missing input logging with debug log (#1121) * Fix CI errors on 1.4.0.7 (#1122) * Fix FailsFV testcase * Don't dispose already disposed runners * Improve error logging in FullNodeFeatureExecutor * Simplify AsyncLock * Fix * Increase timeout * Disable failing Ubuntu and OSX test case temporarily * Stabilize test * Improve test-case * Implement bounded LRU map for peer known inv hashes * Remove spurious 'using' * Make WalletSyncManager more robust * Add fix from upstream NBitcoin * Add missing payload attribute * Don't process deposits prior to already processed deposits (#1124) * Don't process deposits prior to already processed deposits * Fix MaturedBlocksSyncManagerTests * Update src/Stratis.Features.FederatedPeg/TargetChain/MaturedBlocksSyncManager.cs Co-authored-by: zeptin <zeptin@gmail.com> * Fix BlocksAreRequestedIfThereIsSomethingToRequestAsync --------- Co-authored-by: zeptin <zeptin@gmail.com> * Limit how often the wallet is saved (#1125) * Limit how often the wallet is saved * Tweak * Force wallet save in stop * Throw error on duplicate withdrawal detected in federation wallet (#1126) * Limit how often the wallet is saved * Tweak * Force wallet save in stop * Throw error on duplicate withdrawal detected in federation wallet * Stop application * Fix * Just display an error for now * Can only explicitly remove transient transactions * Fix conversion in benchmarks * Add list shuffle helper method * Add P2WSH handling to wallet database * Fix signalr config * Add check for empty list * Extend the PR 624 changes to reward distribution (#1128) * Add maturity height to logging (#1127) * Add maturity height to logging * Restore removed code section * Send potentially suspended tx's to CCTS * Fix * Logging and code improvements * Fix null and enforce sync * Resolve some error scenarios * Cleanup * Remove redundant code (#1129) * Prevent non-ascii characters being omitted from console (#1130) * Use deposit blocktime for conversions (#1142) * Use deposit blocktime for conversions * Fix build errors in tests * Update Launch Scripts * Additional endpoint functionality (#1146) * Check if peer collection modified while trimming (#1143) --------- Co-authored-by: Satya Karki <spacegrlsatya@gmail.com> Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com> Co-authored-by: Nikita Koifman <nik7966@icloud.com> Co-authored-by: noescape0 <NoEscape0@ya.ru> Co-authored-by: quantumagi <someguy.fromafrica@gmail.com> Co-authored-by: Iain McCain <iain.mccain@stratisplatform.com> Co-authored-by: Nikita Koifman <nikita.koifman@stratisplatform.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This federation wallet's transaction processing logic is capable of detecting instances where the wallet already contains withdrawal transactions related to specific deposits. Originally designed to handle transient transactions, it does not currently throw an error when both the existing and new transactions are confirmed withdrawals for the same deposit. This pull request (PR) includes a check for this specific scenario.
It's worth noting that the previous logic would effectively ignore spends from duplicate withdrawals when detecting duplicates. This was based on the assumption that the duplicate transaction would not be a confirmed transaction. However if it is it could result in inputs not being reserved for the duplicate confirmed withdrawal transaction. PR #1124 should shore up the assumption that there would be no duplicate confirmed transactions, but I am adding this check nonetheless.