ingress-egress-tracker: add tx_ref to redis #4573
Merged
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4573 +/- ##
=====================================
- Coverage 73% 72% -0%
=====================================
Files 401 401
Lines 66735 66784 +49
Branches 66735 66784 +49
=====================================
- Hits 48417 48408 -9
- Misses 15974 16026 +52
- Partials 2344 2350 +6 ☔ View full report in Codecov by Sentry. |
niklasnatter
approved these changes
Feb 27, 2024
| enum TransactionRef { | ||
| Bitcoin { hash: String }, | ||
| Ethereum { hash: H256 }, | ||
| Polkadot { transaction_id: PolkadotTransactionId }, |
There was a problem hiding this comment.
how would this get serialized?
marcellorigotti
requested review from
GabrielBuragev and
acdibble
and removed request for
a team
|
We decided to make some changes to the way the tx_ref is reported back to the chain, will update this PR once the other one is up and merged (it is just an implementation detail nothing changes for you @niklasnatter ) |
marcellorigotti
force-pushed
the
feature/pro-1196
branch
from
7f93631 to
3a866df
Compare
|
PR is updated and ready, with the latest changes made to transaction_succeeded extrinsic. |
dandanlen
reviewed
Feb 28, 2024
| #[derive(Serialize)] | ||
| #[serde(untagged)] | ||
| enum TransactionRef { | ||
| Bitcoin { hash: String }, |
There was a problem hiding this comment.
@marcellorigotti
Explicit conversion to String is a bit of an anti-pattern. I think a better solution here is to use BitcoinTransactionHash and define Serialize on that struct. The serialization should reverse the byte order. Then we can use the same struct for the tx_ref and tx_out_id.
IMO this is less confusing than storing two different tx references.
marcellorigotti
requested review from
kylezs and
AlastairHolmes
as code owners
marcellorigotti
requested review from
dandanlen
and removed request for
kylezs and
AlastairHolmes
dandanlen
approved these changes
Mar 1, 2024
syan095
added a commit
that referenced
this pull request
Mar 5, 2024
…utxo * origin/main: fix: upgrade test pnpm install from commit (#4600) chore: add notification on failed release build (#4589) chore: add Zellic audit to Security.md (#4599) update changelog for 1.2.1 (#4597) fix: use correct pnpm deps for upgrade-test (#4596) chore(bouncer): Use stable sdk version (#4595) chore(bouncer): Update sdk to 1.2.1 (#4582) log meaningful message if docker login fails (#4553) Added bouncer command to download a runtime update from a proposal (#4592) chore: add asset to withdrawal event (#4590) ingress-egress-tracker: add tx_ref to redis (#4573) fix: remove tight bound for ingress fee on broker test (#4591) # Conflicts: # state-chain/chains/src/btc.rs
syan095
added a commit
that referenced
this pull request
Mar 7, 2024
…ncoding * origin/main: (21 commits) chore: update chainspecs (#4615) feat: Add channel opening fee to *DepositAddressReady Events (#4609) refactor: pass out CFE incompatibility exit information to main (#4563) feat: Introduce tx fee multiplier storage item (#4594) fix: debug cli (#4605) fix: patch 1.2 broker test (#4607) feat: add block height and deposit details to PrewitnessedDeposit (#4606) chore: add myself as codeowner to upgrade-test (#4603) fix: RUSTSEC-2024-0019 (#4604) fix: upgrade test pnpm install from commit (#4600) chore: add notification on failed release build (#4589) chore: add Zellic audit to Security.md (#4599) update changelog for 1.2.1 (#4597) fix: use correct pnpm deps for upgrade-test (#4596) chore(bouncer): Use stable sdk version (#4595) chore(bouncer): Update sdk to 1.2.1 (#4582) log meaningful message if docker login fails (#4553) Added bouncer command to download a runtime update from a proposal (#4592) chore: add asset to withdrawal event (#4590) ingress-egress-tracker: add tx_ref to redis (#4573) ... # Conflicts: # state-chain/runtime/src/lib.rs
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.
Pull Request
Closes: PRO-1196
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
Added tx_ref to the ingress-egress-tracker
Non-Breaking changes