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: Transaction details page hints #331

Merged
merged 14 commits into from
Jun 29, 2023
Merged

Conversation

janmichek
Copy link
Collaborator

@janmichek janmichek commented Jun 20, 2023

Description

resolves #231

Demo

Checklist:

@github-actions
Copy link

Deployed to https://pr-331-aescan.stg.aepps.com

@janmichek janmichek force-pushed the Transaction_details_page_hints branch from 140a2c0 to ffac685 Compare June 22, 2023 07:59
@janmichek
Copy link
Collaborator Author

@marc0olo I invited you for a review of the next large part of the hints

createdBy: 'Keyblock height and exact date and time when the smart contract was created.',
hash: 'The transaction hash of a ContractCreateTx that was executed to create the smart contract on the blockchain.',
creator: 'Account that created the smart contract on the blockchain.',
status: 'Status of the transaction',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
status: 'Status of the transaction',
status: 'Status of the transaction.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

smartContract: 'Unique identifier for a smart contract instance that lives on the blockchain.',
caller: 'Account that called the smart contract.',
amount: 'Amount of AE coins that was transferred to smart contract address.',
entryPoint: 'The entry point of the smart contract that has been called.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
entryPoint: 'The entry point of the smart contract that has been called.',
entryPoint: 'The entrypoint of the smart contract that has been called.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed all over the app

caller: 'Account that called the smart contract.',
amount: 'Amount of AE coins that was transferred to smart contract address.',
entryPoint: 'The entry point of the smart contract that has been called.',
arguments: 'Arguments that was passed to entry point call with the call.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
arguments: 'Arguments that was passed to entry point call with the call.',
arguments: 'Arguments that were passed to the entrypoint.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

amount: 'Amount of AE coins that was transferred to smart contract address.',
entryPoint: 'The entry point of the smart contract that has been called.',
arguments: 'Arguments that was passed to entry point call with the call.',
return: 'Value that has been emitted after the call to entry point.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return: 'Value that has been emitted after the call to entry point.',
return: 'Value that was returned by the entrypoint.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

entryPoint: 'The entry point of the smart contract that has been called.',
arguments: 'Arguments that was passed to entry point call with the call.',
return: 'Value that has been emitted after the call to entry point.',
gasLimit: 'Maximum amount of gas that will be paid for transaction.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gasLimit: 'Maximum amount of gas that will be paid for transaction.',
gasLimit: 'Maximum amount of gas to be consumed by this transaction.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

gaAttachTxStatus: 'Status of the transaction.',
gaAttachTxAccount: 'Account that was generalized.',
gaAttachTxSmartContractId: 'Unique identifier of the smart contract that was attached to the account.',
gaAttachTxGasLimit: 'Maximum amount of gas that will be paid for transaction.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gaAttachTxGasLimit: 'Maximum amount of gas that will be paid for transaction.',
gaAttachTxGasLimit: 'Maximum amount of gas to be consumed by this transaction.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

gaAttachTxAccount: 'Account that was generalized.',
gaAttachTxSmartContractId: 'Unique identifier of the smart contract that was attached to the account.',
gaAttachTxGasLimit: 'Maximum amount of gas that will be paid for transaction.',
gaAttachTxAuthFunction: 'Name of the smart contract function that was called to generalize the account.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gaAttachTxAuthFunction: 'Name of the smart contract function that was called to generalize the account.',
gaAttachTxAuthFunction: 'Name of the smart contract entrypoint that in the future handles the authorization logic of the account.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

gaAttachTxSmartContractId: 'Unique identifier of the smart contract that was attached to the account.',
gaAttachTxGasLimit: 'Maximum amount of gas that will be paid for transaction.',
gaAttachTxAuthFunction: 'Name of the smart contract function that was called to generalize the account.',
gaAttachTxArguments: 'Arguments that were passed to the smart contract call.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gaAttachTxArguments: 'Arguments that were passed to the smart contract call.',
gaAttachTxArguments: 'Arguments that were passed to initialize the attached smart contract.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

gaAttachTxAuthFunction: 'Name of the smart contract function that was called to generalize the account.',
gaAttachTxArguments: 'Arguments that were passed to the smart contract call.',
gaAttachTxGasPrice: 'Price for one unit of gas.',
gaAttachTxGasUsed: 'Amount of gas that was paid for interaction with the smart contract.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gaAttachTxGasUsed: 'Amount of gas that was paid for interaction with the smart contract.',
gaAttachTxGasUsed: 'Amount of gas that was consumed for attaching the smart contract to the account.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

gaAttachTxArguments: 'Arguments that were passed to the smart contract call.',
gaAttachTxGasPrice: 'Price for one unit of gas.',
gaAttachTxGasUsed: 'Amount of gas that was paid for interaction with the smart contract.',
gaAttachTxGasCost: 'Price in AE that was paid for interaction with the smart contract.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gaAttachTxGasCost: 'Price in AE that was paid for interaction with the smart contract.',
gaAttachTxGasCost: 'The AE paid for attaching the smart contract to the account.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@marc0olo
Copy link
Contributor

@janmichek for the state channel tx-hints I think it would be good to let @velzevur and/or @uwiger have a look. I actually didn't review the state channel hints so far.

@janmichek
Copy link
Collaborator Author

@janmichek for the state channel tx-hints I think it would be good to let @velzevur and/or @uwiger have a look. I actually didn't review the state channel hints so far.

Thanks for the great suggestions, all accepted.
Yes please, that would be awesome. @velzevur @uwiger feel free to collaborate by providing the code review .)

@marc0olo
Copy link
Contributor

@velzevur @uwiger: this is the file you need to check (you can ignore the rest I think 😅)

https://github.com/aeternity/aescan/pull/331/files#diff-720acf0511e27a8fa6ee7a840566647bf27e3896a32a9c2e8ec1c2b5f2b489e2

Copy link
Contributor

@lukeromanowicz lukeromanowicz left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me =)

@janmichek janmichek requested review from uwiger and velzevur June 28, 2023 09:41
responderCloseAmount: 'Final balance for the responder.',
responderSettleAmount: 'Unsigned amount of coins the responder gets from the channel.',
settledBy: 'Participant of the channel that posts the settling transaction.',
snapshotRound: 'Channel\'s next round.',

Choose a reason for hiding this comment

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

I am not sure if this is a bit misleading and it could be misleading, maybe try:

Suggested change
snapshotRound: 'Channel\'s next round.',
snapshotRound: 'Channel snapshot\'s round.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

accepted

slashedRound: 'The round in which the slash was triggered.',
slashedBy: 'The account that slashed (disputed) malicious ChannelCloseSoloTx or ChannelForceProgressTx.',
initiatorCreateAmount: 'Unsigned amount of coins the initiator commits to the channel.',
initiatorCloseAmount: 'Final balance for the initiator.',

Choose a reason for hiding this comment

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

What is the difference between initiatorCloseAmount and initiatorSettleAmount? In what context are they being used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

forcedBy: 'Participant or a delegate of the channel that posts the force progress transaction.',
depositor: 'Account that deposited funds to the state channel.',
depositRound: 'The channel\'s internal round that applies the deposit.',
depositAmount: 'Amount deposited to the state channels.',

Choose a reason for hiding this comment

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

Suggested change
depositAmount: 'Amount deposited to the state channels.',
depositAmount: 'Amount deposited to the state channel.',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

closeRound: 'The round in which the state channel was proposed to be closed.',
closeProposedBy: 'The participant of the channel that posts the closing transaction.',
sentBy: 'State channel participant or delegate that posts the slashing transaction.',
channelReserve: 'The minimum amount of AE coins that must remain locked in the channel.',

Choose a reason for hiding this comment

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

Suggested change
channelReserve: 'The minimum amount of AE coins that must remain locked in the channel.',
channelReserve: 'The minimum amount of AE coins that must remain locked in any of the off-chain accounts in the channel.',

It is not the total amount locked in the channel but a minimum threshold for both accounts independently

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, accepted

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Good job, I left some suggestions.

src/utils/hints/contractsHints.js Outdated Show resolved Hide resolved
src/utils/hints/namesHints.js Outdated Show resolved Hide resolved
src/utils/hints/namesHints.js Outdated Show resolved Hide resolved
src/utils/hints/namesHints.js Outdated Show resolved Hide resolved
src/utils/hints/oraclesHints.js Outdated Show resolved Hide resolved
src/utils/hints/stateChannelsHints.js Outdated Show resolved Hide resolved
src/utils/hints/stateChannelsHints.js Outdated Show resolved Hide resolved
src/utils/hints/stateChannelsHints.js Outdated Show resolved Hide resolved
src/utils/hints/stateChannelsHints.js Outdated Show resolved Hide resolved
src/utils/hints/stateChannelsHints.js Outdated Show resolved Hide resolved
@janmichek
Copy link
Collaborator Author

Good job, I left some suggestions.

Thank you, all fixed

Copy link
Collaborator

@michele-franchi michele-franchi 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 fixes and the really good job. 👏

@janmichek janmichek merged commit 635e6ac into develop Jun 29, 2023
@janmichek janmichek deleted the Transaction_details_page_hints branch June 29, 2023 09:11
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.

Transaction details page hints
6 participants