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

CCTP and Stride protos #10988

Merged
merged 5 commits into from
Feb 13, 2025
Merged

CCTP and Stride protos #10988

merged 5 commits into from
Feb 13, 2025

Conversation

turadg
Copy link
Member

@turadg turadg commented Feb 11, 2025

incidental

Description

Adds CCTP and Stride message codecs to support upcoming work.

Stride protos pulled from master...99adarsh:agoric-sdk:stride-elys by @99adarsh

Security Considerations

no provenance info for Stride protos. @99adarsh can you explain how to confirm they're correct?

Scaling Considerations

This shouldn't affect other imports unless they use the barrel, and on-chain code doesn't.

Documentation Considerations

nothing special

Testing Considerations

mechanical, nothing to test

Upgrade Considerations

not on chain

@turadg turadg requested a review from 0xpatrickdev February 11, 2025 22:49
@turadg turadg requested a review from a team as a code owner February 11, 2025 22:49
Copy link

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

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: d3c8d99
Status: ✅  Deploy successful!
Preview URL: https://0ebcbf5b.agoric-sdk.pages.dev
Branch Preview URL: https://ta-protos.agoric-sdk.pages.dev

View logs

Copy link
Member

@0xpatrickdev 0xpatrickdev left a comment

Choose a reason for hiding this comment

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

Looks good, consider including a git ref/tag in the commit message or PR body for the stride protos

}

// Adds a new oracle
message MsgAddOracle {
Copy link
Member

Choose a reason for hiding this comment

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

I think these are the first app/chain-specific protos we're adding to this package. There seems to be a lot of non-user facing stuff in here. There's probably limited harm to including everything, but making sure it's a cognizant decision to do so.

A quick sweep it seems these are essential:

  • stakeibc, staketia, stakedym and their dependencies records, epochs

These also seem to be user facing, but not needed for the .liquidStake story:

  • airdrop, autopilot, claim, vesting

If anything, maybe just remove icaoracle, interchainquery, mint (if it doesn't affect codegen via import deps)

Copy link
Member Author

Choose a reason for hiding this comment

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

Only what ends up in the package exports will be used and I had only added stride/stakeibc/tx. But better not to include files that aren't exported. I'll keep it to staking packages (and deps) but I'll wait for @99adarsh to chime in before I merge.

Copy link
Contributor

Choose a reason for hiding this comment

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

@turadg @0xpatrickdev keeping only stakeibc, staketia, stakedym and their dep makes sense.

@99adarsh
Copy link
Contributor

99adarsh commented Feb 12, 2025

Hey @turadg , I pulled the protos from stride's main branch. You can verify this from here https://github.com/Stride-Labs/stride/tree/main/proto/stride , Commit 41eb417

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Feb 12, 2025
@@ -96,6 +100,10 @@
"types": "./dist/codegen/icq/v1/packet.d.ts",
"default": "./dist/codegen/icq/v1/packet.js"
},
"./stride/stakeibc/tx.js": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@turadg I think, we should also add stride/staketia/tx and stride/stakedym/tx as some of it will be used in the future.

@turadg turadg force-pushed the ta/protos branch 2 times, most recently from f62f840 to d720009 Compare February 12, 2025 18:41
@turadg turadg requested a review from 0xpatrickdev February 13, 2025 17:57
@turadg turadg added the bypass:integration Prevent integration tests from running on PR label Feb 13, 2025
@turadg
Copy link
Member Author

turadg commented Feb 13, 2025

Added bypass:integration because it was green on top of master yesterday and nothing here affects integration tests.

@mergify mergify bot merged commit 729fe8e into master Feb 13, 2025
83 of 93 checks passed
@mergify mergify bot deleted the ta/protos branch February 13, 2025 18:24
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 bypass:integration Prevent integration tests from running on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants