fix(Dataworker): Dereference originChainId by index, not key #936
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
Temporary fix to resolve an issue with future quoteTimestamps. We need a more robust solution than this in the long run.
pxrl
requested review from
mrice32,
nicholaspai and
james-a-morris
as code owners
|
Hm; this is actually failing for me in local test, but I believe this fix is just unearthing a separate issue. |
src/dataworker/PoolRebalanceUtils.ts
Outdated
| @@ -84,17 +84,21 @@ export function updateRunningBalanceForEarlyDeposit( | |||
| deposit: typechain.FundsDepositedEvent, | |||
| updateAmount: BigNumber | |||
| ): void { | |||
| // deposit.args is a pure array; there are no mapping to be dereferenced. | |||
| const originChainId = deposit.args[1].toNumber(); | |||
There was a problem hiding this comment.
We may want to wrap this as Number(String(deposit.args[1])) to ensure that we won't get a null reference. Alternatively - we can ensure that deposit.args[1] !== undefined
There was a problem hiding this comment.
I think .toString() will work on a BigNumber, not sure about String()
mrice32
approved these changes
Sep 11, 2023
Signed-off-by: Matt Rice <[email protected]>
nicholaspai
pushed a commit
that referenced
this pull request
Sep 20, 2023
* fix(Dataworker): Dereference originChainId by index, not key Temporary fix to resolve an issue with future quoteTimestamps. We need a more robust solution than this in the long run. * Extract originToken from array * lint * WIP Signed-off-by: Matt Rice <[email protected]> --------- Signed-off-by: Matt Rice <[email protected]> Co-authored-by: Matt Rice <[email protected]>
nicholaspai
added a commit
that referenced
this pull request
Sep 28, 2023
…tamps (#944) * improve(dataworker): Remove `transferThreshold` Fixes ACX-1537 Removes a lot of code needed to support deprecated config store variable that is always set to 0 in production * Update Dataworker.ts * bumpity * Update Dataworker.ts * Update package.json * Update yarn.lock * wip * fix(Dataworker): Dereference originChainId by index, not key (#936) * fix(Dataworker): Dereference originChainId by index, not key Temporary fix to resolve an issue with future quoteTimestamps. We need a more robust solution than this in the long run. * Extract originToken from array * lint * WIP Signed-off-by: Matt Rice <[email protected]> --------- Signed-off-by: Matt Rice <[email protected]> Co-authored-by: Matt Rice <[email protected]> * improve: leverage default TTL & leverage caching (#938) * improve: leverage default TTL * improve: set fn signature to default * feat: add dataworker force bundle range (#912) * feat: add dataworker force bundle range * docs: add documentation to example env * improve: allow for boba * improve: allow forced proposal range with sending --------- Co-authored-by: Paul <[email protected]> * feat: test chain routes (#897) * feat: Script for automating SpokePool deposits This is useful for making deposits to the zkSync SpokePool(s) because zkSync's block explorer doesn't yet support making transactions via proxy contracts. * Add yarn script target + tweak usage * Add basic support for dumping SpokePool config * lint * fix: Gracefully handle token searches where token missing Not all tokens are defined on all chains, so don't try to drop case unless the token actually exists on a chain. * Add "fetch" for dumping deposit and fill information Currently limitations: - Does not display updated values (i.e. after a speed-up). - Does not display message data. - Should normalise from the token decimals for better readability. - Does not gracefully handle when the txnHash is not found. * lint * fix conflict * lint * chore: allow base-units to easily be sent * chore: allow token to be passed as origin token addr or symbol * feat: test chain routes * nit: improve docs * docs: add docs * improve: account for checksummed value Co-authored-by: Paul <[email protected]> * improve: remove unneeded flag Co-authored-by: Paul <[email protected]> * improve: docs Co-authored-by: Paul <[email protected]> * improve: do not hardcode amounts * improve: modification * improve: dynamically compute price --------- Co-authored-by: Paul <[email protected]> Co-authored-by: nicholaspai <[email protected]> * WIP * Update yarn.lock * Revert "WIP" This reverts commit 381e5bf. * feat: Add protection against computing LP fees for future quote timestamps This resolves the [reverted SDK PR](across-protocol/sdk#399) and re-introduces protection from deposit quote timestamps that are impossible to compute. This PR essentially moves that protection to the production version of Hub and Spoke clients so that the unit tests can continue to work and not have to add additional logic to set quote times correctly. Ideally in the future we set the quote times how they would in production but for now this solution results in a much smaller code diff * lint * fix * lint * Update HubPoolClient.ts * refactor: Import getNetworkName from sdk (#945) * chore: bump sdk to latest (#943) * chore: bump sdk to latest * chore: bump version * chore: bump sdk --------- Signed-off-by: Matt Rice <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Matt Rice <[email protected]> Co-authored-by: James Morris, MS <[email protected]>
nicholaspai
added a commit
that referenced
this pull request
Sep 29, 2023
* improve(dataworker): Remove `transferThreshold` Fixes ACX-1537 Removes a lot of code needed to support deprecated config store variable that is always set to 0 in production * Update Dataworker.ts * Update Dataworker.ts * Update package.json * Update yarn.lock * wip * WIP * Update yarn.lock * Revert "WIP" This reverts commit 381e5bf. * chore: bump sdk to latest (#943) * chore: bump sdk to latest * chore: bump version * chore: bump sdk * Update src/dataworker/Dataworker.ts Co-authored-by: Paul <[email protected]> * feat: Add protection against computing LP fees for future quote timestamps (#944) * improve(dataworker): Remove `transferThreshold` Fixes ACX-1537 Removes a lot of code needed to support deprecated config store variable that is always set to 0 in production * Update Dataworker.ts * bumpity * Update Dataworker.ts * Update package.json * Update yarn.lock * wip * fix(Dataworker): Dereference originChainId by index, not key (#936) * fix(Dataworker): Dereference originChainId by index, not key Temporary fix to resolve an issue with future quoteTimestamps. We need a more robust solution than this in the long run. * Extract originToken from array * lint * WIP Signed-off-by: Matt Rice <[email protected]> --------- Signed-off-by: Matt Rice <[email protected]> Co-authored-by: Matt Rice <[email protected]> * improve: leverage default TTL & leverage caching (#938) * improve: leverage default TTL * improve: set fn signature to default * feat: add dataworker force bundle range (#912) * feat: add dataworker force bundle range * docs: add documentation to example env * improve: allow for boba * improve: allow forced proposal range with sending --------- Co-authored-by: Paul <[email protected]> * feat: test chain routes (#897) * feat: Script for automating SpokePool deposits This is useful for making deposits to the zkSync SpokePool(s) because zkSync's block explorer doesn't yet support making transactions via proxy contracts. * Add yarn script target + tweak usage * Add basic support for dumping SpokePool config * lint * fix: Gracefully handle token searches where token missing Not all tokens are defined on all chains, so don't try to drop case unless the token actually exists on a chain. * Add "fetch" for dumping deposit and fill information Currently limitations: - Does not display updated values (i.e. after a speed-up). - Does not display message data. - Should normalise from the token decimals for better readability. - Does not gracefully handle when the txnHash is not found. * lint * fix conflict * lint * chore: allow base-units to easily be sent * chore: allow token to be passed as origin token addr or symbol * feat: test chain routes * nit: improve docs * docs: add docs * improve: account for checksummed value Co-authored-by: Paul <[email protected]> * improve: remove unneeded flag Co-authored-by: Paul <[email protected]> * improve: docs Co-authored-by: Paul <[email protected]> * improve: do not hardcode amounts * improve: modification * improve: dynamically compute price --------- Co-authored-by: Paul <[email protected]> Co-authored-by: nicholaspai <[email protected]> * WIP * Update yarn.lock * Revert "WIP" This reverts commit 381e5bf. * feat: Add protection against computing LP fees for future quote timestamps This resolves the [reverted SDK PR](across-protocol/sdk#399) and re-introduces protection from deposit quote timestamps that are impossible to compute. This PR essentially moves that protection to the production version of Hub and Spoke clients so that the unit tests can continue to work and not have to add additional logic to set quote times correctly. Ideally in the future we set the quote times how they would in production but for now this solution results in a much smaller code diff * lint * fix * lint * Update HubPoolClient.ts * refactor: Import getNetworkName from sdk (#945) * chore: bump sdk to latest (#943) * chore: bump sdk to latest * chore: bump version * chore: bump sdk --------- Signed-off-by: Matt Rice <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Matt Rice <[email protected]> Co-authored-by: James Morris, MS <[email protected]> --------- Signed-off-by: Matt Rice <[email protected]> Co-authored-by: James Morris, MS <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Matt Rice <[email protected]>
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.
Temporary fix to resolve an issue with future quoteTimestamps. We need a more robust solution than this in the long run.