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

9207 orchestrator stub #9208

Merged
merged 5 commits into from
Apr 24, 2024
Merged

9207 orchestrator stub #9208

merged 5 commits into from
Apr 24, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Apr 8, 2024

step towards: #9207

Description

Adapt https://github.com/agoric-labs/orchestration-api-spec/ into agoric-sdk

Once approved, I'll clean up the commits.

Once landed, I'll archive the other repo, pointing people to @agoric/orchestration in this codebase.

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

@0xpatrickdev 0xpatrickdev force-pushed the 8881-ica-send-message branch 2 times, most recently from 38c7bc3 to 54d830f Compare April 9, 2024 17:50
Base automatically changed from 8881-ica-send-message to master April 9, 2024 18:46
Copy link

cloudflare-workers-and-pages bot commented Apr 22, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1e054ac
Status: ✅  Deploy successful!
Preview URL: https://5f2979f4.agoric-sdk.pages.dev
Branch Preview URL: https://9207-orchestrator-stub.agoric-sdk.pages.dev

View logs

@turadg turadg force-pushed the 9207-orchestrator-stub branch 2 times, most recently from e449107 to f808615 Compare April 23, 2024 20:45
@turadg turadg marked this pull request as ready for review April 24, 2024 00:06
/**
* Submit a transaction on behalf of the remote accoutn for execution on teh remote chain.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Submit a transaction on behalf of the remote accoutn for execution on teh remote chain.
* Submit a transaction on behalf of the remote account for execution on the remote chain.

@@ -93,6 +93,8 @@ export const prepareStakingAccountHolder = (baggage, makeRecorderKit, zcf) => {
getUpdater() {
return this.state.topicKit.recorder;
},
// TODO move this beneath the Orchestration abstraction,
// to the OrchestrationAccount provided by makeAccount()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// to the OrchestrationAccount provided by makeAccount()
// to the OrchestrationAccount provided by createAccount()

@@ -0,0 +1,102 @@
// @ts-check
Copy link
Member

Choose a reason for hiding this comment

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

Per feedback in #9198 it's maybe best to move these to /tools or /examples. I am planning to move the current contents of /contracts to /examples in that PR to avoid potential confusion

Copy link
Member Author

Choose a reason for hiding this comment

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

I think whatever we do should be consistent across the repository. I'm going to leave this for a later PR in which we execute a plan wholistically

Comment on lines +53 to +56
const [undelegation] = await celestiaAccount.undelegate(delegations);

// wait for the undelegations to be complete (may take weeks)
await undelegation.completion;
Copy link
Member

Choose a reason for hiding this comment

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

👍 highlighting this change for other reviewers

import type { TimestampRecord } from '@agoric/time';
import type { ChainAddress } from './service.js';

export type Delegation = {
Copy link
Member

@0xpatrickdev 0xpatrickdev Apr 24, 2024

Choose a reason for hiding this comment

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

Suggested change
export type Delegation = {
TODO import from `@agoric/cosmic-proto` instead of reimplementing
export type Delegation = {

/**
* Register a hook to intercept an incoming IBC Transfer and handle it.
* Calling without arguments will unregister the hook.
*/
Copy link
Member

@0xpatrickdev 0xpatrickdev Apr 24, 2024

Choose a reason for hiding this comment

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

Suggested change
*/
TODO reference type from #8624 `packages/vats/src/localchain.js`
*/

}

export interface Undelegation {
cancel: () => Promise<void>;
Copy link
Member

@0xpatrickdev 0xpatrickdev Apr 24, 2024

Choose a reason for hiding this comment

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

Suggested change
cancel: () => Promise<void>;
cancel: () => Promise<MsgCancelUnbondingDelegationResponse>;

Would also need to add this type to delegation.d.ts:

type MsgCancelUnbondingDelegationResponse {}; 

This type/message does not appear in our current @agoric/cosmic-proto, and was seemingly released in cosmos-sdk v0.46.x

Comment on lines 266 to 278
/**
* A LocalChain account that has the ability to intercept IBC Transfer
* packets and react to them. a.k.a. "IBC Hooks"
*/

export interface TransferAccount extends LocalChainAccount {
/**
* Register a hook to intercept an incoming IBC Transfer.
* Calling without arguments will unregister the hook
*/
interceptTransfer: (tap?: { upcall: (args: any) => Promise<any> }) => void;
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/**
* A LocalChain account that has the ability to intercept IBC Transfer
* packets and react to them. a.k.a. "IBC Hooks"
*/
export interface TransferAccount extends LocalChainAccount {
/**
* Register a hook to intercept an incoming IBC Transfer.
* Calling without arguments will unregister the hook
*/
interceptTransfer: (tap?: { upcall: (args: any) => Promise<any> }) => void;
}

Please delete, this was removed in this commit in favor of exposing interceptTransfer under agoric on KnownChains

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Apr 24, 2024
@mergify mergify bot merged commit 76b8d53 into master Apr 24, 2024
75 checks passed
@mergify mergify bot deleted the 9207-orchestrator-stub branch April 24, 2024 16:33
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.

2 participants