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

Support all relevant transaction types in trace account tool #78

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

simonkamp
Copy link
Contributor

Purpose

Closes #77

The trace account tool should support all transactions types with an associated cost.

Changes

Made the tool robust against rejected transactions, which previously crashed it.

Added support for all update/add/remove/deploy/register transaction types.

Added support for transferWithMemo, encryptedAmountTransferWithMemo, and transferWithScheduleAndMemo transactions, by using their counterparts without a memo.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • I have updated the CHANGELOG.

Copy link
Contributor

@abizjak abizjak left a comment

Choose a reason for hiding this comment

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

Let's talk about this when you are in next week.

It is getting a bit unwieldy.

@@ -144,15 +145,18 @@ enum AdditionalDetails {
InitContract,
#[serde(rename = "update")]
Update,
#[serde(rename = "transfer")]
#[serde(rename = "transfer", alias = "transferWithMemo")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I know alias is convenient, but it is conceptually "wrong", so please add a separate TransferWithMemo variant.

#[serde(rename = "encryptedAmountTransfer")]
#[serde(
rename = "encryptedAmountTransfer",
alias = "encryptedAmountTransferWithMemo"
Copy link
Contributor

Choose a reason for hiding this comment

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

And equivalently here.

EncryptedAmountTransfer(EncryptedTransfer),
#[serde(rename = "transferToEncrypted")]
TransferToEncrypted(TransferToEncrypted),
#[serde(rename = "transferToPublic")]
TransferToPublic(TransferToPublic),
#[serde(rename = "transferWithSchedule")]
#[serde(rename = "transferWithSchedule", alias = "transferWithScheduleAndMemo")]
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

of {} GTUs\n Block hash: {}\n Transaction hash: {}",
pretty_time(tx.block_time),
i,
match tx.details.additional_details {// todo can any of the transactions below affect other accounts?
Copy link
Contributor

Choose a reason for hiding this comment

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

No, they just affect the sender account.

Bargsteen added a commit that referenced this pull request Sep 13, 2023
Move `ExchangeRates` from concordium-std
Bargsteen added a commit that referenced this pull request Sep 14, 2023
Move `ExchangeRates` from concordium-std
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.

support all transaction types
2 participants