-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
4859e99
to
754acf5
Compare
091259c
to
d1d07d7
Compare
055c2be
to
2d9c3ce
Compare
2d9c3ce
to
8abc111
Compare
8abc111
to
f99d33b
Compare
"last 1 safari version" | ||
] | ||
} | ||
"browserslist": [ |
There was a problem hiding this comment.
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)
There was a problem hiding this 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.
}); | ||
} | ||
|
||
// TODO identify that attribute is a token and show as NFT, in case it is. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
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. |
@@ -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", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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'
}
src/components/tx/TxData.js
Outdated
this.setState({ ncLoading: true }); | ||
if (this.props.transaction.version !== hathorLib.constants.NANO_CONTRACTS_VERSION) { | ||
this.setState({ ncLoading: false }); | ||
return; | ||
} |
There was a problem hiding this comment.
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.
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 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense: 8bdd32b
Acceptance Criteria
TxData
component when using theisBlock
method. This PR also fixes it.nano.explorer.mov
Edit on the screen:
Security Checklist