chore: tx statuses in vstorage #10750
Closed
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
- Advancing txs are dequeueable so `case PendingTxStatus.Advancing:` is reachable - update Settler tests to use `notifyAdvancing` result to simulate an advance - add Settler test for minting while advancing (Advanced + AdvanceFailed paths)
- if Forward (slow transfer) fails, capture the state to distinguish from successful forwards
- Advancer calls `forwardIfMinted` to check for an early matching mint - if found, no Advance occurs and the minted funds are forwarded
- ensure "minted before observed" `Observe` statuses are recorded in vstorage - ensure "minted while Advancing" `Advanced` and `AdvanceFailed` statuses are recorded in vstorage
Deploying agoric-sdk with
|
| Latest commit: |
c7db794
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f763db2a.agoric-sdk.pages.dev |
| Branch Preview URL: | https://pc-10625-vstorage-writes.agoric-sdk.pages.dev |
turadg
reviewed
Dec 20, 2024
| * @param {EvmHash} txHash | ||
| * @param {boolean} success whether the Transfer succeeded | ||
| */ | ||
| notifyAdvanceOutcome(txHash, success) { |
There was a problem hiding this comment.
This name is awkward since advanceOutcome is already "notifying".
I get that the existing one is keyed by (nfa, amount) and this is keyed by txHash. So the existing one is for "pending" transactions and this new one is for settled.
Line two of your comment draws the contrast being "settled". Let's just use that in the name. Something like,
Suggested change
| notifyAdvanceOutcome(txHash, success) { | |
| advanceOutcomeForSettled(txHash, success) { |
0xpatrickdev
force-pushed
the
pc/10625-forward-failed
branch
2 times, most recently
from
08bf844 to
3a0afe7
Compare
|
Closing in favor of #10729 |
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.
refs: #10625
Description
Observestatuses are recorded in vstorageAdvancedandAdvanceFailedstatuses are recorded in vstorageSecurity Considerations
None, improves observability off chain.
Scaling Considerations
None
Documentation Considerations
None
Testing Considerations
Updates existing tests
Upgrade Considerations
Targeting initial FUSDC release