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

feature/PRO-1712/reject-tx #5332

Merged
merged 74 commits into from
Oct 29, 2024
Merged

feature/PRO-1712/reject-tx #5332

merged 74 commits into from
Oct 29, 2024

Conversation

Janislav
Copy link
Contributor

@Janislav Janislav commented Oct 15, 2024

Pull Request

Closes: PRO-1712

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have written sufficient tests.
  • I have written and tested required migrations.
  • I have updated documentation where appropriate.

Summary

Please include a succinct description of the purpose and content of the PR. What problem does it solve, and how? Link issues, discussions, other PRs, and anything else that will help the reviewer.

Non-Breaking changes

If this PR includes non-breaking changes, select the non-breaking label. On merge, CI will automatically cherry-pick the commit to a PR against the release branch.

- Extended DepositChannelDetails with owner field
- Added extrinsic to mark transaction as tainted
- Handling deposit and save details for a refund if tx is tainted
- Added tests to verify that tainted transactions can get detected for all possible swap types
- Added tests  to check that txs marked by other brokers are getting ignored
- Taking refund address if we open a deposit channel as lp
- Extended ChannelAction to take an optional refund address
- Removed all dependencies regarding the LpRegistration trait (added last commit)
- Refactored tests, benchmarks, etc
- Removed not needed derives
- Clippy
@Janislav Janislav marked this pull request as ready for review October 24, 2024 14:53
Janislav and others added 4 commits October 28, 2024 11:44
- Fire event if we can not reject tx
- Split amount and fees in trait
- Fix UTXO construct
- use Utxo and remove BtcDepositDetails
- use saturating_sub for egress fee subtraction
- use ChainAccount instead of ForeignChainAddress in RejectCall
- simplify definitions of RejectCall using default method
@dandanlen dandanlen requested review from kylezs and a team as code owners October 29, 2024 11:35
@dandanlen dandanlen requested review from zoheb391 and acdibble and removed request for a team, zoheb391 and acdibble October 29, 2024 11:35
@dandanlen dandanlen enabled auto-merge October 29, 2024 15:21
@dandanlen dandanlen added this pull request to the merge queue Oct 29, 2024
Merged via the queue into main with commit 079a149 Oct 29, 2024
48 of 49 checks passed
@dandanlen dandanlen deleted the feature/pro-1712/reject-tx branch October 29, 2024 16:47
dandanlen added a commit that referenced this pull request Oct 31, 2024
* feat: Handling of tainted transactions

- Extended DepositChannelDetails with owner field
- Added extrinsic to mark transaction as tainted
- Handling deposit and save details for a refund if tx is tainted

* tests: Added tests

- Added tests to verify that tainted transactions can get detected for all possible swap types
- Added tests  to check that txs marked by other brokers are getting ignored

* chore: Extended LpRegistration trait + Added to Ingress/Egress config

* feat: Getting LP refund address

* feature: ensure by lp and broker + added more tests

* refactor: Don't error if tx is tainted

* refactor: Using DoubleMap instead of Map

* refactor: Using BadOrigin + Added unit test

* refactor: Inline code + Add Deposit Witness to struct

* refactor: Extended lp deposit with refund address

- Taking refund address if we open a deposit channel as lp
- Extended ChannelAction to take an optional refund address
- Removed all dependencies regarding the LpRegistration trait (added last commit)
- Refactored tests, benchmarks, etc

* feature: Added benchmark

* chore: Changes to benchmark

* chore: generated mock weights

* chore: Added POC for BTC swap rejecting.

* chore: only allow mark transactions for BTC

* feature: expire tainted transaction

* test: refactored tests

* chore: Extended RejectCall with DepositWitness

* Revert "chore: Extended RejectCall with DepositWitness"

This reverts commit 67873ff.

* chore: added draft for eth refund implementation

* chore: Removed unused events

* chore: Ensure only by broker

* chore: removed broker from tainted tx struct

* chore: Only clone owner

* chore: Moved tx tainted check

* feature: Added migration for DepositChannelLookup

* refactor: Changed data structure + fixed migrations

* chore: Handle LP refund address as requirement

* chore: Made clippy happy 🙂

* chore: don't manipulate storage in place in iteration 🙅‍♂️

* test: Added migration test 🧪

* chore: changed pallet storage version 📀

* chore: bumped pallet storage version (again)

* chore: using ScheduledTxForReject for refunding

* refactor: Changed accounting of expired transactions

* refactor: using translate for migration

* refactor: using append, refactored test

* feature: Added handling of boost channels

* feature: Marking txs when prewitness and reject when we process the depo

* feat: pre-witnessed rejection handling

* chore: Fixed logic + added tests

* tests: Refactor/Rearranged tests

* chore: Using SECONDS_PER_BLOCK instead of static block seconds

* chore: Addressed comments

* chore: Fixed clippy in CI

* chore: update comments

* fix: don't allow report overwrite

* chore: Renamed event

* feat: improvements:

- mark boosted transactions as boosted instead of using channel status
- allow pallet config instead of relying on chain
- add event for tx reports
- only allow reporting of unseen transactions
- add doc comments
- renaming of types/events
- remove unused error

* chore: removed not needed RefundParameters, added test

* refactor: Rejecting

- Changed trait
- Refactor AllBatch build
- LogOrPanic if reject failed and safe details

* chore: fix compiler errors

* chore: subtract fees

* fix: using deposit address

* fix: addressed problems

- Introduced NoChangeTransfer ApiCall
- Fire event when tx was broadcasted
- Handle on_broadcast_ready with no-op

* chore: small improvements

- Removed not needed derives
- Clippy

* wip: change depositDetails for btc to utxo

* Revert "wip: change depositDetails for btc to utxo"

This reverts commit 55de221.

* refactor: added current status of tx_id refactor

* tests: Moved screening feature tests to own file and test against btc

* refactor:

- Fire event if we can not reject tx
- Split amount and fees in trait
- Fix UTXO construct

* chore: rearranged imports

* refactors:

- use Utxo and remove BtcDepositDetails
- use saturating_sub for egress fee subtraction
- use ChainAccount instead of ForeignChainAddress in RejectCall
- simplify definitions of RejectCall using default method

---------

Co-authored-by: Daniel <daniel@chainflip.io>
Co-authored-by: Maxim Shishmarev <maxim@chainflip.io>
dandanlen added a commit that referenced this pull request Oct 31, 2024
* feat: Handling of tainted transactions

- Extended DepositChannelDetails with owner field
- Added extrinsic to mark transaction as tainted
- Handling deposit and save details for a refund if tx is tainted

* tests: Added tests

- Added tests to verify that tainted transactions can get detected for all possible swap types
- Added tests  to check that txs marked by other brokers are getting ignored

* chore: Extended LpRegistration trait + Added to Ingress/Egress config

* feat: Getting LP refund address

* feature: ensure by lp and broker + added more tests

* refactor: Don't error if tx is tainted

* refactor: Using DoubleMap instead of Map

* refactor: Using BadOrigin + Added unit test

* refactor: Inline code + Add Deposit Witness to struct

* refactor: Extended lp deposit with refund address

- Taking refund address if we open a deposit channel as lp
- Extended ChannelAction to take an optional refund address
- Removed all dependencies regarding the LpRegistration trait (added last commit)
- Refactored tests, benchmarks, etc

* feature: Added benchmark

* chore: Changes to benchmark

* chore: generated mock weights

* chore: Added POC for BTC swap rejecting.

* chore: only allow mark transactions for BTC

* feature: expire tainted transaction

* test: refactored tests

* chore: Extended RejectCall with DepositWitness

* Revert "chore: Extended RejectCall with DepositWitness"

This reverts commit 67873ff.

* chore: added draft for eth refund implementation

* chore: Removed unused events

* chore: Ensure only by broker

* chore: removed broker from tainted tx struct

* chore: Only clone owner

* chore: Moved tx tainted check

* feature: Added migration for DepositChannelLookup

* refactor: Changed data structure + fixed migrations

* chore: Handle LP refund address as requirement

* chore: Made clippy happy 🙂

* chore: don't manipulate storage in place in iteration 🙅‍♂️

* test: Added migration test 🧪

* chore: changed pallet storage version 📀

* chore: bumped pallet storage version (again)

* chore: using ScheduledTxForReject for refunding

* refactor: Changed accounting of expired transactions

* refactor: using translate for migration

* refactor: using append, refactored test

* feature: Added handling of boost channels

* feature: Marking txs when prewitness and reject when we process the depo

* feat: pre-witnessed rejection handling

* chore: Fixed logic + added tests

* tests: Refactor/Rearranged tests

* chore: Using SECONDS_PER_BLOCK instead of static block seconds

* chore: Addressed comments

* chore: Fixed clippy in CI

* chore: update comments

* fix: don't allow report overwrite

* chore: Renamed event

* feat: improvements:

- mark boosted transactions as boosted instead of using channel status
- allow pallet config instead of relying on chain
- add event for tx reports
- only allow reporting of unseen transactions
- add doc comments
- renaming of types/events
- remove unused error

* chore: removed not needed RefundParameters, added test

* refactor: Rejecting

- Changed trait
- Refactor AllBatch build
- LogOrPanic if reject failed and safe details

* chore: fix compiler errors

* chore: subtract fees

* fix: using deposit address

* fix: addressed problems

- Introduced NoChangeTransfer ApiCall
- Fire event when tx was broadcasted
- Handle on_broadcast_ready with no-op

* chore: small improvements

- Removed not needed derives
- Clippy

* wip: change depositDetails for btc to utxo

* Revert "wip: change depositDetails for btc to utxo"

This reverts commit 55de221.

* refactor: added current status of tx_id refactor

* tests: Moved screening feature tests to own file and test against btc

* refactor:

- Fire event if we can not reject tx
- Split amount and fees in trait
- Fix UTXO construct

* chore: rearranged imports

* refactors:

- use Utxo and remove BtcDepositDetails
- use saturating_sub for egress fee subtraction
- use ChainAccount instead of ForeignChainAddress in RejectCall
- simplify definitions of RejectCall using default method

---------

Co-authored-by: Daniel <daniel@chainflip.io>
Co-authored-by: Maxim Shishmarev <maxim@chainflip.io>
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