-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix(sdk): expose transaction ref from ingress egress tracker in sdk swaps [WEB-1643] #1102
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1102 +/- ##
==========================================
- Coverage 87.21% 87.18% -0.03%
==========================================
Files 100 100
Lines 1369 1366 -3
Branches 214 214
==========================================
- Hits 1194 1191 -3
Misses 166 166
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
const parsedDeposit = depositSchema.parse(deposit); | ||
const { deposit_details, deposit_chain_block_height } = parsedDeposit; | ||
|
||
const baseDeposit = { | ||
amount: parsedDeposit.amount, | ||
asset: parsedDeposit.asset, | ||
deposit_chain_block_height: parsedDeposit.deposit_chain_block_height, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a little shorter:
const parsedDeposit = depositSchema.parse(deposit); | |
const { deposit_details, deposit_chain_block_height } = parsedDeposit; | |
const baseDeposit = { | |
amount: parsedDeposit.amount, | |
asset: parsedDeposit.asset, | |
deposit_chain_block_height: parsedDeposit.deposit_chain_block_height, | |
}; | |
const { deposit_details, ...baseDeposit } = depositSchema.parse(deposit); | |
const { deposit_chain_block_height } = baseDeposit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if i need to somehow movebaseDeposit
to the depositSchema
(or i need to remove baseDeposit
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, fixed it 🤦
}, | ||
]); | ||
expect(mock).toHaveBeenCalledWith('deposit:Ethereum:0x1234', 0, -1); | ||
}); | ||
|
||
it('returns the deposits if found - Polkadot', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@@ -12,14 +13,38 @@ const jsonString = string.transform((value) => JSON.parse(value)); | |||
|
|||
const chainAsset = uncheckedAssetAndChain.transform(({ asset }) => asset); | |||
|
|||
const BitcoinDeposit = z.object({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these can be camelCase
const BitcoinDeposit = z.object({ | |
const bitcoinDeposit = z.object({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
|
||
if (!deposit_details) return { ...baseDeposit, tx_refs: [] }; | ||
|
||
switch (chain) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we maybe reuse the getDepositTxRef
method here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been trying to reuse that function but it didn't really work because the shape of the ingress egress tracker does not match the depositDetails
types (bitcoinSchema160
, bitcoinSchema170
...). I'm going to go back to my previous commits
@@ -127,12 +152,36 @@ export default class RedisClient { | |||
return value ? broadcastParsers[chain].parse(JSON.parse(value)) : null; | |||
} | |||
|
|||
async getDeposits(chain: Chain, asset: Asset, address: string) { | |||
async getDeposits(chain: Chain, asset: Asset, address: string): Promise<PendingDeposit[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that this method returns the tx ref information, we should return it from the status
endpoint. to do that, we need to adjust the getPendingDeposit
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but would be fine for me to do that in a separate pr if you prefer that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do in separate pr 👾
I'll try to refactor |
2f668b3
to
f2e6025
Compare
…waps [WEB-1643] (#1102) * fix(swap sdk): expose transaction ref from ingress egress tracker in sdk status * fix: clean * fix: clean * fix getDeposit func * fix types * add test and clean * fix swap files * fix imports * fix pipeline * camelcases
Added
deposit_details
field ondepositSchema
from the backend