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

docs: Modify Governor whitepaper to include information about Flow Cancelling #3982

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

johnsaigle
Copy link
Contributor

A documentation PR that adds information on Flow Cancel Tokens added in #3798.

Please let me know if the level of detail is sufficient or if it would be better to move the text to another section.

bruce-riley
bruce-riley previously approved these changes Jun 14, 2024
djb15
djb15 previously approved these changes Jun 17, 2024
SEJeff
SEJeff previously approved these changes Jun 18, 2024
whitepapers/0007_governor.md Outdated Show resolved Hide resolved
whitepapers/0007_governor.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

The *Configuration*: heading should get a new bullet point for flow cancel.

#### Headroom Calculations

The headroom for a given chain is the sum of the notional USD value of all transfers of governed tokens emitted from that chain within a 24 hour sliding window.
Inbound transfers of certain tokens can also decrease this sum, a process we refer to as Flow Canceling. The tokens are listed in [flow_cancel_tokens.go](https://github.com/wormhole-foundation/wormhole/blob/main/node/pkg/governor/flow_cancel_tokens.go). An inbound transfer of these tokens to chain will reduce that chain's outbound limit: effectively the net-flow is zero. This allows for a relaxing of the Governor's rate-limiting as it accounts for the net flow of these assets rather than calculating only the outbound value.
Copy link
Contributor

Choose a reason for hiding this comment

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

An inbound transfer of these tokens to chain will reduce that chain's outbound limit: effectively the net-flow is zero.

This sentence structure is odd and "these tokens to chain will..." sounds grammatically incorrect. Additionally, this does not reduce the chain's outbound limit but rather the calculated aggregate.

@@ -69,7 +74,7 @@ Each Guardian publishes its Governor configuration and status on the Wormhole go

## Detailed Design

The Governor is implemented as an additional package that defines (1) a `ChainGovernor` object, (2) `mainnet_tokens.go`, a single map of tokens that will be monitored, and (3) `mainnet_chains.go`, a map of chains governed by the chain governor.
The Governor is implemented as an additional package that defines (1) a `ChainGovernor` object, (2) `mainnet_tokens.go`, a single map of tokens that will be monitored, (3) `mainnet_chains.go`, a map of chains governed by the chain governor, and (4) `flow_cancel_tokens.go`, a map of tokens that can reduce the Governor's rate limit.
Copy link
Contributor

@evan-gray evan-gray Jul 1, 2024

Choose a reason for hiding this comment

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

can reduce the Governor's rate limit.

nit: "can reduce a chains calculated aggregate flow." or some term that is more accurate than "reduce the ... limit" since the limit remains the same.

@johnsaigle johnsaigle self-assigned this Jul 2, 2024
@johnsaigle johnsaigle marked this pull request as draft July 17, 2024 13:27
@johnsaigle
Copy link
Contributor Author

Converting to draft until #4016 is settled and merged. Once we have a flow cancel redeployed and new considerations are accounted for, I'll come back to this document and make sure it reflects the changes that we've made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants