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

feat(tendermint): staking/delegation #2322

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Jan 15, 2025

Adds tendermint protocol support on add_delegation RPC, and extends tendermint transaction history implementation to support delegation transactions.

Next is to do the same for undelegation on remove_delegation.

TODO: File a documentation issue on https://github.com/KomodoPlatform/komodo-docs-mdx

@onur-ozkan onur-ozkan changed the title feat(tendermint): delegate/undelegate RPCs feat(tendermint): staking/delegation Jan 15, 2025
@onur-ozkan onur-ozkan force-pushed the tendermint-delegation branch from 081df90 to 4012550 Compare January 15, 2025 14:46
@onur-ozkan onur-ozkan force-pushed the tendermint-delegation branch 4 times, most recently from 8145b90 to 16dad85 Compare January 16, 2025 10:20
Signed-off-by: onur-ozkan <[email protected]>
@onur-ozkan onur-ozkan force-pushed the tendermint-delegation branch 2 times, most recently from f67185a to 1dd2b73 Compare January 16, 2025 10:47
@onur-ozkan onur-ozkan force-pushed the tendermint-delegation branch from 4340e37 to 0f08e05 Compare January 20, 2025 10:07
@onur-ozkan onur-ozkan marked this pull request as ready for review January 20, 2025 10:07
Copy link
Member

@laruh laruh left a comment

Choose a reason for hiding this comment

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

Thank you! code is very clean, I have small notes

mm2src/coins/tendermint/tendermint_coin.rs Show resolved Hide resolved
mm2src/coins/tendermint/tendermint_coin.rs Outdated Show resolved Hide resolved
@shamardy shamardy self-requested a review January 23, 2025 12:06
shamardy
shamardy previously approved these changes Jan 23, 2025
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thanks for the clean and easy to review PR! Only one non-blocker.

Copy link
Member

@laruh laruh left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@borngraced borngraced left a comment

Choose a reason for hiding this comment

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

Great work...one not directly related note

&account_info,
fee,
timeout_height,
memo.clone(),
Copy link
Member

@borngraced borngraced Jan 24, 2025

Choose a reason for hiding this comment

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

not directly related but I think it worth mention that we can avoid cloning memo everywhere.
We can pass ref str to tx::Body::new(vec![tx_payload], &memo, timeout_height as u32); like here
Screenshot 2025-01-24 at 08 07 16

&self,
withdraw_from: Option<WithdrawFrom>,
) -> Result<(AccountId, Option<H256>), WithdrawError> {
) -> Result<(AccountId, Option<H256>), String> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we not generally striving to avoid String errors (in favour of always typed errors)?

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

Successfully merging this pull request may close these issues.

5 participants