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

feat(fast-usdc): skipAdvance when preconditions fail #11005

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Conversation

samsiegart
Copy link
Contributor

closes #10994

Description

If any of the preconditions throw for making the advance, set the state to ADVANCE_SKIPPED rather than OBSERVED. This makes it more obvious that the contract has actively chosen to not advance, rather than getting stuck on OBSERVED for some reason.

Security Considerations

N/A

Scaling Considerations

N/A

Documentation Considerations

I don't think we have any docs that go this granular on the flow.

Testing Considerations

Updated tests

Upgrade Considerations

N/A

@samsiegart samsiegart requested a review from a team as a code owner February 13, 2025 22:51
Copy link

cloudflare-workers-and-pages bot commented Feb 13, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: cfe0954
Status: ✅  Deploy successful!
Preview URL: https://6a3265ba.agoric-sdk.pages.dev
Branch Preview URL: https://srs-skip-advance.agoric-sdk.pages.dev

View logs

@@ -217,7 +217,7 @@ export const prepareAdvancerKit = (
});
} catch (error) {
log('Advancer error:', error);
statusManager.observe(evidence);
statusManager.skipAdvance(evidence, ['PRECONDITIONS_FAILED']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. As discussed offline let's try this approach, like smart wallet:

Suggested change
statusManager.skipAdvance(evidence, ['PRECONDITIONS_FAILED']);
statusManager.skipAdvance(evidence, [error.message]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@samsiegart samsiegart added the automerge:rebase Automatically rebase updates, then merge label Feb 14, 2025
Copy link
Contributor

mergify bot commented Feb 14, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

Copy link
Contributor

mergify bot commented Feb 18, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@mergify mergify bot merged commit 7b582c1 into master Feb 18, 2025
83 checks passed
@mergify mergify bot deleted the srs-skip-advance branch February 18, 2025 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use skipAdvance rather than observe when precondition checks fail
2 participants