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

chore(settler): narrow "unexpected channel" log #11046

Closed
wants to merge 1 commit into from

Conversation

0xpatrickdev
Copy link
Member

@0xpatrickdev 0xpatrickdev commented Feb 24, 2025

closes: #11047

Description

receiveUpcall fires on incoming and outgoing ibc transfers. This change quiets the "unexpected channel" log on outgoing transfers sent by the contract's settlementAccount. (The settlementAccount will send an outgoing transfer during the Forward sequence.)

Security Considerations

None, diagnostic-only change.

Scaling Considerations

Less noise in SwingSet logs

Documentation Considerations

None

Testing Considerations

None

Upgrade Considerations

This is a breaking change for SettlerKit / makeSettler, via the addition of a new state property in state and stateShape.

- `receiveUpcall` fires on incoming and outgoing ibc transfers. quiet the "unexpected channel" log on outgoing transfers sent by the contract
@0xpatrickdev 0xpatrickdev requested a review from a team as a code owner February 24, 2025 21:44
@0xpatrickdev 0xpatrickdev added the force:integration Force integration tests to run on PR label Feb 24, 2025
@0xpatrickdev 0xpatrickdev requested review from turadg and dckc February 24, 2025 21:46
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

please add more detail to upgrade considerations

@@ -97,6 +97,7 @@ export const stateShape = harden({
settlementAccount: M.remotable('Account'),
registration: M.or(M.undefined(), M.remotable('Registration')),
sourceChannel: M.string(),
destChannel: M.string(),
Copy link
Member

Choose a reason for hiding this comment

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

hm. state shape change.

I suppose this is OK because we're making a new settler next time.

Please elaborate the upgrade considerations to be explicit about this.

@@ -160,6 +161,7 @@ export const prepareSettler = (
/**
* @param {{
* sourceChannel: IBCChannelID;
* destChannel: IBCChannelID;
Copy link
Member

Choose a reason for hiding this comment

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

This (and the fact that tests had to change) looks like a breaking change to the makeSettler API.

Again, I think it's fine because we're making a new settler, but please be explicit in the upgrade considerations.

@0xpatrickdev
Copy link
Member Author

Closing as this was deemed P2 - let's track this under #11047 if we want to pick it up. It appears to depend on #10200.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fusdc: quiet "unexpected channel" log on outgoing forwards
2 participants