refactor(vats): Rename vtransfer TransferInterceptorKit facets by their role #3
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
This was referenced Jun 7, 2024
gibson042
force-pushed
the
gibson-9059-vtransfer-interceptor-refactor
branch
3 times, most recently
from
f33c681
to
d720fe0
Compare
michaelfig
force-pushed
the
vtransfer
branch
2 times, most recently
from
fb21f7e
to
95d81c2
Compare
closes: #8583, closes: #9059, closes: #9256 ## Description This PR adds the vTransfer middleware module which acts as a notification module on the transfer port for contracts that need to act based on it. ### Security Considerations As discussed, this does change how inbound assets are handled. ### Testing Considerations The middleware module has mock unit tests in ibc_middleware_test.go
closes: #XXXX refs: #9465 ## Description This PR just captures the test from #9465 , turning it into an expected failure. A later PR that fixes the bug would then remove the `.failing`. ### Security Considerations The bug itself endangers integrity, so fixing it will help security. This PR though just adds the test that demonstrates the bug. ### Scaling Considerations none ### Documentation Considerations none ### Testing Considerations The point. ### Upgrade Considerations The bug means that an asyncFlow activation that does return early would be committed to the done state, preventing a later upgrade from repairing it to get beyond the problem. But this PR just adds the test for that bug.
more accurate. What's built is a code/permit pair (and its bundles), not a proposal (which has a title and a list of core evals)
## Description I was recently confused for the nth time on why writeCoreProposal wasn't writing a proposal. Now that we model proposals in `a3p-integration` (something proposed to stakers) I've been running into this confusion more often. [CoreEvalProposal](https://github.com/Agoric/agoric-sdk/blob/4491bd134d194ba5a965a6b0ff9f394563581162/golang/cosmos/x/swingset/types/swingset.pb.go#L28-L37) has a title and `evals` which isis an array of [CoreEval](https://github.com/Agoric/agoric-sdk/blob/4491bd134d194ba5a965a6b0ff9f394563581162/golang/cosmos/x/swingset/types/swingset.pb.go#L72-L81). That's the thing that has permits and code, that `writeCoreProposal` is writing out. This names what it's creating `CoreEval`. It's not exactly the CoreEval type but it's 1:1 with a CoreEval so I think it's warranted. ### Security Considerations n/a ### Scaling Considerations n/a ### Documentation Considerations Improves clarity. I don't know what external docs need to be updated. ### Testing Considerations CI should suffice ### Upgrade Considerations It should be fully backwards compatible as I maintained the module exports. I renamed a module but it shouldn't have been deep imported anywhere.
- update CosmosChainInfoShape to match type: stakingTokens is optional
- add IST to vbankAsset - inspectLocalBridge
closes #9187 ## Description DRAFT until - [x] what to do about the way `transfer()` maps chainId to transferChannel? - [x] clean up git history - [x] update other facade callers - [x] integrate more fully with well-known chains: hot-new-chain gets added and "Nothing breaks." - [x] what to do with facade contract upgrade test, now that chainInfos is a caller thing? ### Security Considerations <!-- Does this change introduce new assumptions or dependencies that, if violated, could introduce security vulnerabilities? How does this PR change the boundaries between mutually-suspicious components? What new authorities are introduced by this change, perhaps by new API calls? --> ### Scaling Considerations <!-- Does this change require or encourage significant increase in consumption of CPU cycles, RAM, on-chain storage, message exchanges, or other scarce resources? If so, can that be prevented or mitigated? --> ### Documentation Considerations <!-- Give our docs folks some hints about what needs to be described to downstream users. Backwards compatibility: what happens to existing data or deployments when this code is shipped? Do we need to instruct users to do something to upgrade their saved data? If there is no upgrade path possible, how bad will that be for users? --> ### Testing Considerations <!-- Every PR should of course come with tests of its own functionality. What additional tests are still needed beyond those unit tests? How does this affect CI, other test automation, or the testnet? --> ### Upgrade Considerations <!-- What aspects of this PR are relevant to upgrading live production systems, and how should they be addressed? -->
closes: #XXXX refs: #9299 #9322 ## Description Prepare for #9322 by making any guest use of `E` until then cause an error. We expect that it might be a while before #9322 is ready for review. By merging this PR soon, we prevent any guest code or logs that would commit us to an incompat way of handling `E`. ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations Should document that guests cannot invoke host vows or remotables with `E` until #9322 , which won't happen immediately. ### Testing Considerations - [x] need to test what kind of error state this goes into. Should be a panic, so that an upgrade can unblock guest execution that got stuck trying to `E`. ### Upgrade Considerations The point. By making such use of `E` an error now, we ensure that #9322 can proceed without causing any compat problem with committed durable state. Co-authored-by: Mathieu Hofman <[email protected]>
michaelfig
force-pushed
the
gibson-9059-vtransfer-interceptor-refactor
branch
from
d720fe0
to
8d11989
Compare
|
Superseded by Agoric#9470 |
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.
Agoric#8624 (comment)