Skip to content

Validate cross-chain deposit target#561

Merged
quantumagi merged 19 commits intostratisproject:masterfrom
quantumagi:validatecctx
May 25, 2021
Merged

Validate cross-chain deposit target#561
quantumagi merged 19 commits intostratisproject:masterfrom
quantumagi:validatecctx

Conversation

@quantumagi
Copy link
Contributor

@quantumagi quantumagi commented May 19, 2021 •
edited
Loading

There is more going on in this PR than one would expect so here are the reasons for the various changes:

  1. Moves OpReturnDataReader to where it can be re-used. The legacy class takes a CounterChainNetworkWrapper argument but that is not known within the class's new context, so its changed to take a Network argument.
  2. Moves the re-usable bits, used when extracting deposits, to a helper class.
  3. Adds a bare-bones network class used specifically when validating Cirrus addresses. Due to circular references it's difficult to include the standard network classes, hence this solution.
  4. Adds a validation method to the helper class and calls that from the BuildTransaction methods used by Cirrus and Strax.

@quantumagi quantumagi changed the title Validate cross-chain transaction target Validate cross-chain deposit target May 19, 2021
@quantumagi quantumagi marked this pull request as ready for review May 20, 2021 06:22
@quantumagi quantumagi requested a review from fassadlr May 20, 2021 06:23
fassadlr
fassadlr reviewed May 24, 2021
View reviewed changes
src/Stratis.Bitcoin.Features.Wallet/OpReturnDataReader.cs Outdated
using TracerAttributes;

namespace Stratis.Features.FederatedPeg
namespace Stratis.Bitcoin.Features.Wallet
Copy link
Contributor

@fassadlr fassadlr May 24, 2021

Choose a reason for hiding this comment

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

Im thinking this might better places in Stratis.Bitcoin so we dont create more project dependencies? Perhaps in a folder under Stratis.Bitcoin called Sidechain or CrossChain?

IMO it is a core class and generic enough to be put there rather. Thoughts?

Copy link
Contributor Author

@quantumagi quantumagi May 24, 2021

Choose a reason for hiding this comment

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

Done.

@quantumagi quantumagi merged commit 4491a0f into stratisproject:master May 25, 2021
zeptin pushed a commit to zeptin/StratisFullNode that referenced this pull request Jun 15, 2021
@quantumagi @zeptin
Validate cross-chain deposit target (stratisproject#561)
54eea1c 
* Validate cross-chain transaction target

* Add more changes

* Change validation network

* Refactor

* Add ValidateCrossChainTransferAddress method

* Rename method

* Add some comments

* Update tests

* Add validation to SmartContractTransactionService

* Add minimum

* Fix spelling

* Fix message

* Update CirrusAddressValidationNetwork

* Add comments

* Refactor

* Remove whitespace

* Test for federation

* Move OpReturnDataReader to Stratis.Bitcoin

* Add reference
fassadlr pushed a commit that referenced this pull request Jul 1, 2021
@quantumagi @fassadlr
Validate cross-chain deposit target (#561)
9ea75e5 
* Validate cross-chain transaction target

* Add more changes

* Change validation network

* Refactor

* Add ValidateCrossChainTransferAddress method

* Rename method

* Add some comments

* Update tests

* Add validation to SmartContractTransactionService

* Add minimum

* Fix spelling

* Fix message

* Update CirrusAddressValidationNetwork

* Add comments

* Refactor

* Remove whitespace

* Test for federation

* Move OpReturnDataReader to Stratis.Bitcoin

* Add reference
fassadlr pushed a commit to fassadlr/StratisFullNode that referenced this pull request Jul 2, 2021
@quantumagi @fassadlr
Validate cross-chain deposit target (stratisproject#561)
af5ec31 
* Validate cross-chain transaction target

* Add more changes

* Change validation network

* Refactor

* Add ValidateCrossChainTransferAddress method

* Rename method

* Add some comments

* Update tests

* Add validation to SmartContractTransactionService

* Add minimum

* Fix spelling

* Fix message

* Update CirrusAddressValidationNetwork

* Add comments

* Refactor

* Remove whitespace

* Test for federation

* Move OpReturnDataReader to Stratis.Bitcoin

* Add reference
fassadlr added a commit to fassadlr/StratisFullNode that referenced this pull request Jul 2, 2021
@fassadlr
Fix Merge of stratisproject#561
88ee25e
fassadlr added a commit that referenced this pull request Jul 7, 2021
@fassadlr @quantumagi
[Interflux] Fixes based on stress test / Merge validate cross chain t…
6ee7f93 
…ransfer target address (#604)

* Stress test fixes / InteropBehavior should reply to attached peer / Console Logging

* Validate cross-chain deposit target (#561)

* Validate cross-chain transaction target

* Add more changes

* Change validation network

* Refactor

* Add ValidateCrossChainTransferAddress method

* Rename method

* Add some comments

* Update tests

* Add validation to SmartContractTransactionService

* Add minimum

* Fix spelling

* Fix message

* Update CirrusAddressValidationNetwork

* Add comments

* Refactor

* Remove whitespace

* Test for federation

* Move OpReturnDataReader to Stratis.Bitcoin

* Add reference

* Fix Merge of #561

* Pre Build

* Revert Ethereum Address check to be in line with initial interflux branch

* Lessen the submission tx confirmation threshold

* Fix NodeStats DI

* Fix Null requestEthTransactionHash

* Remove log

* Update InteropPoller.cs

* Fix serialization and check if request finalized

* Fix Build

* Dont serialize Eth Tx Hash

* Refactor to request / reply model

* Revert threshold

* Fix originator logic

* Add Ethereum address check to InterFluxOpReturnEncoder (#601)

* Fix build from merge

* [Interflux] Explicitly specify Enum values for DestinationChain (#602)

* Explicitly specify Enum values for DestinationChain

* Fix order

* Self review

Co-authored-by: quantumagi <someguy.fromafrica@gmail.com>
fassadlr added a commit that referenced this pull request Jul 13, 2021
@fassadlr @quantumagi @zeptin
Release/1.0.9.1 (#620)
27793e9 
* Bump Nuget versions

* Validate cross-chain deposit target (#561)

* Validate cross-chain transaction target

* Add more changes

* Change validation network

* Refactor

* Add ValidateCrossChainTransferAddress method

* Rename method

* Add some comments

* Update tests

* Add validation to SmartContractTransactionService

* Add minimum

* Fix spelling

* Fix message

* Update CirrusAddressValidationNetwork

* Add comments

* Refactor

* Remove whitespace

* Test for federation

* Move OpReturnDataReader to Stratis.Bitcoin

* Add reference

* Add Ethereum address check to InterFluxOpReturnEncoder (#601)

* Bump version to 1.0.9.1

* [Interflux] Explicitly specify Enum values for DestinationChain (#602)

* Explicitly specify Enum values for DestinationChain

* Fix order

* [InterFlux] Only allow ETH Interflux conversions (#608)

* Only allow ETH Interflux conversions

* Update src/Stratis.Bitcoin.Features.Wallet/Controllers/WalletController.cs

Co-authored-by: zeptin <zeptin@gmail.com>

Co-authored-by: zeptin <zeptin@gmail.com>

* Add Ethereum address check to InterFluxOpReturnEncoder (#601)

* Support legacy ETH conversion OP_RETURNs (#615)

* Fix SmartContract History (#596)

* Fix SmartContract History

* Update GetHistory

* Update GetHistory

* Update GetHistory

* Update query

* Update query

* Fix column name

* Add address filter to wallet service

* Fix query

* Refactor

* Use SC ScriptAddressReader

* Refactor

Co-authored-by: quantumagi <someguy.fromafrica@gmail.com>
Co-authored-by: zeptin <zeptin@gmail.com>
fassadlr added a commit that referenced this pull request Jul 23, 2021
@fassadlr @quantumagi
@zeptin @StratisIain @rowandh
Merge Release/1.0.9.1 into SidechainMasternode-RC (#642)
8c5d577 
* Bump Nuget versions

* Validate cross-chain deposit target (#561)

* Validate cross-chain transaction target

* Add more changes

* Change validation network

* Refactor

* Add ValidateCrossChainTransferAddress method

* Rename method

* Add some comments

* Update tests

* Add validation to SmartContractTransactionService

* Add minimum

* Fix spelling

* Fix message

* Update CirrusAddressValidationNetwork

* Add comments

* Refactor

* Remove whitespace

* Test for federation

* Move OpReturnDataReader to Stratis.Bitcoin

* Add reference

* Add Ethereum address check to InterFluxOpReturnEncoder (#601)

* Bump version to 1.0.9.1

* [Interflux] Explicitly specify Enum values for DestinationChain (#602)

* Explicitly specify Enum values for DestinationChain

* Fix order

* [InterFlux] Only allow ETH Interflux conversions (#608)

* Only allow ETH Interflux conversions

* Update src/Stratis.Bitcoin.Features.Wallet/Controllers/WalletController.cs

Co-authored-by: zeptin <zeptin@gmail.com>

Co-authored-by: zeptin <zeptin@gmail.com>

* Add Ethereum address check to InterFluxOpReturnEncoder (#601)

* Support legacy ETH conversion OP_RETURNs (#615)

* Fix SmartContract History (#596)

* Fix SmartContract History

* Update GetHistory

* Update GetHistory

* Update GetHistory

* Update query

* Update query

* Fix column name

* Add address filter to wallet service

* Fix query

* Refactor

* Use SC ScriptAddressReader

* Refactor

* Update network check points (#619)

* Update network check points

* Fix tests

* Only reserve UTXOs once the transaction has been built successfully. (#618)

* Update StraxTest.cs (#630)

* Update launch script (#638)

* Remove Password Check and GETH Check

* Signed Script

* Contract dynamic swagger endpoint (#527)

Co-authored-by: quantumagi <someguy.fromafrica@gmail.com>
Co-authored-by: zeptin <zeptin@gmail.com>
Co-authored-by: StratisIain <32906542+StratisIain@users.noreply.github.com>
Co-authored-by: Rowan de Haas <rowandh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@fassadlr fassadlr fassadlr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@quantumagi @fassadlr