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: add support for nano contract details #253

Merged
merged 14 commits into from
Apr 12, 2024
Merged

Conversation

pedroferreira1
Copy link
Member

@pedroferreira1 pedroferreira1 commented Dec 7, 2023

Acceptance Criteria

  • Add screen to show the details of a nano contract with state and history (TODO the history table still doesn't have pagination but this will be added in the next version).
  • Add screen to show details of a blueprint.
  • Show nano contract information in the Transaction Detail screen.
  • It seems like there's a bug in the TxData component when using the isBlock method. This PR also fixes it.
nano.explorer.mov

Edit on the screen:

image

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@pedroferreira1 pedroferreira1 self-assigned this Dec 7, 2023
@pedroferreira1 pedroferreira1 changed the base branch from feat/new-lib-version to refactor/new-lib-version December 7, 2023 22:38
Base automatically changed from refactor/new-lib-version to dev February 6, 2024 01:39
"last 1 safari version"
]
}
"browserslist": [
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this I was getting an error when running locally (with node 18)

Copy link
Member

@r4mmer r4mmer left a comment

Choose a reason for hiding this comment

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

For the blueprint detail screen I found it somewhat weird to have a table where one of the cells has a table with the arguments.
Maybe we could show the arguments as a method signature, for instance:

name arguments return
initialize (oracle_script: bytes, token_uid: bytes, date_last_bet: int) null

Or something like:

initialize(oracle_script: bytes, token_uid: bytes, date_last_bet: int): None

For each method.

src/App.js Show resolved Hide resolved
src/components/tx/TxData.js Show resolved Hide resolved
src/components/tx/TxData.js Outdated Show resolved Hide resolved
});
}

// TODO identify that attribute is a token and show as NFT, in case it is.
Copy link
Member

Choose a reason for hiding this comment

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

This TODO is for another PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Right now we don't have the information when an attribute is a token. I discussed this with Marcelo and we will improve this on hathor-core, so we can better render information on the client.

Copy link
Contributor

@tuliomir tuliomir left a comment

Choose a reason for hiding this comment

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

Although it's the nature of the Explorer app to just display data schemas fetched from outside, I felt there could be more references to what data is being displayed here.

This is especially important when displaying the data on the NanoContractDetail screen. I think adding some docstrings and explanatory comments that reference the original schemes and designs would be of great help for future code maintenance.

src/App.js Show resolved Hide resolved
src/api/nanoApi.js Show resolved Hide resolved
src/components/tx/TxData.js Show resolved Hide resolved
src/screens/nano/BlueprintDetail.js Outdated Show resolved Hide resolved
@pedroferreira1
Copy link
Member Author

pedroferreira1 commented Apr 9, 2024

@r4mmer

For the blueprint detail screen I found it somewhat weird to have a table where one of the cells has a table with the arguments.
Maybe we could show the arguments as a method signature, for instance:

I agree, it's much better, see: b31843f

image

@pedroferreira1
Copy link
Member Author

@tuliomir

This is especially important when displaying the data on the NanoContractDetail screen. I think adding some docstrings and explanatory comments that reference the original schemes and designs would be of great help for future code maintenance.

I think having the docs on the APIs is enough. We get the data to show on the screen from the APIs and the user can see it there.

Soon the nano APIs will be in the official API docs, then I can add the links in the code as well.

tuliomir
tuliomir previously approved these changes Apr 9, 2024
r4mmer
r4mmer previously approved these changes Apr 12, 2024
@@ -34,23 +34,17 @@
"scripts": {
"build-css": "sass src:src",
"watch-css": "npm run build-css && sass --watch src:src",
"start-js": "react-scripts start",
"start-js": "react-scripts --openssl-legacy-provider start",
Copy link
Member

Choose a reason for hiding this comment

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

This is only required for node 20 (I think) are we upgrading the required node?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the explorer we use node 18 and without this we get an error:

Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:69:19)
    at Object.createHash (node:crypto:133:10)
    at module.exports (/Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/webpack/lib/util/createHash.js:135:53)
    at NormalModule._initBuildHash (/Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/webpack/lib/NormalModule.js:417:16)
    at /Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/webpack/lib/NormalModule.js:452:10
    at /Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/webpack/lib/NormalModule.js:323:13
    at /Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/loader-runner/lib/LoaderRunner.js:367:11
    at /Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/loader-runner/lib/LoaderRunner.js:233:18
    at context.callback (/Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
    at /Users/pedroferreira/Hathor/nano-hathor-explorer/hathor-explorer/node_modules/babel-loader/lib/index.js:59:103 {
  opensslErrorStack: [ 'error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

Comment on lines 94 to 98
this.setState({ ncLoading: true });
if (this.props.transaction.version !== hathorLib.constants.NANO_CONTRACTS_VERSION) {
this.setState({ ncLoading: false });
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

There is a possibility of the "nano contract data loading" message flashing for transactions that are not nano contracts, maybe we should use this.

Suggested change
this.setState({ ncLoading: true });
if (this.props.transaction.version !== hathorLib.constants.NANO_CONTRACTS_VERSION) {
this.setState({ ncLoading: false });
return;
}
if (this.props.transaction.version !== hathorLib.constants.NANO_CONTRACTS_VERSION) {
this.setState({ ncLoading: false });
return;
}
this.setState({ ncLoading: true });

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense: 8bdd32b

@pedroferreira1 pedroferreira1 dismissed stale reviews from r4mmer and tuliomir via 8bdd32b April 12, 2024 20:35
@pedroferreira1 pedroferreira1 merged commit e991544 into dev Apr 12, 2024
1 check passed
@pedroferreira1 pedroferreira1 deleted the feat/nano-details branch April 12, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants