-
Notifications
You must be signed in to change notification settings - Fork 98
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
Billy/454 tx info #770
Billy/454 tx info #770
Changes from 20 commits
09fa294
0f799d6
171b219
ae37dc7
a2d926b
79d0d76
a05d8ba
d0c3184
5189ebc
eb783f0
933b571
a5299cc
2408361
7c82076
57d3a35
9965edf
b016e6b
945fb68
d2ed57f
3eeeec3
e3fcb7c
94c167f
1373730
86e36e7
87b1375
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,6 +89,8 @@ Object.assign(Client.prototype, { | |
}, | ||
coinTxs: argReq("GET", "/tx/coin"), | ||
|
||
txs: argReq("GET", "/txs"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🥇 |
||
|
||
// staking | ||
candidate: argReq("GET", "/query/stake/candidate"), | ||
candidates: req("GET", "/query/stake/candidates"), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import createHash from "create-hash" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't we just get this from the Node crypto module to avoid adding a dependency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice, didn't realize it was also that ez |
||
import varint from "varint" | ||
import b64 from "base64-js" | ||
|
||
export default function(txstring) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The script is called utils and exports a function that converts tx strings into json? I think this is not really understandable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree it should be named, could be something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense, i think |
||
let txbytes = b64.toByteArray(txstring) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add a dependency instead of doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. didn't realize it was that ez! |
||
let varintlen = new Uint8Array(varint.encode(txbytes.length)) | ||
let tmp = new Uint8Array(varintlen.byteLength + txbytes.byteLength) | ||
tmp.set(new Uint8Array(varintlen), 0) | ||
tmp.set(new Uint8Array(txbytes), varintlen.byteLength) | ||
return createHash("ripemd160") | ||
.update(Buffer.from(tmp)) | ||
.digest("hex") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
import convertTx from "../../scripts/utils.js" | ||
|
||
export default ({ commit, node }) => { | ||
const state = { | ||
blocks: [], | ||
|
@@ -7,7 +9,8 @@ export default ({ commit, node }) => { | |
blockLoading: false, | ||
subscription: false, | ||
syncing: true, | ||
blockMetas: [] | ||
blockMetas: [], | ||
blockTxs: [] | ||
} | ||
|
||
const mutations = { | ||
|
@@ -19,6 +22,9 @@ export default ({ commit, node }) => { | |
}, | ||
setBlockMetaInfo(state, blockMetaInfo) { | ||
state.blockMetaInfo = blockMetaInfo | ||
}, | ||
setBlockTxInfo(state, blockTxInfo) { | ||
state.blockTxInfo = blockTxInfo | ||
} | ||
} | ||
|
||
|
@@ -30,21 +36,33 @@ export default ({ commit, node }) => { | |
dispatch("subscribeToBlocks") | ||
}, | ||
async getBlock({ state, commit, dispatch }, height) { | ||
state.blockLoading = true | ||
state.blockHeight = height | ||
return Promise.all([ | ||
dispatch("queryBlock", height).then(block => commit("setBlock", block)), | ||
dispatch("queryBlockInfo", height).then(blockMetaInfo => | ||
commit("setBlockMetaInfo", blockMetaInfo) | ||
) | ||
]).then( | ||
() => { | ||
state.blockLoading = false | ||
}, | ||
() => { | ||
state.blockLoading = false | ||
} | ||
) | ||
return new Promise((resolve, reject) => { | ||
state.blockLoading = true | ||
state.blockHeight = height | ||
return Promise.all([ | ||
dispatch("queryBlock", height).then(block => | ||
commit("setBlock", block) | ||
), | ||
dispatch("queryBlockInfo", height).then(blockMetaInfo => | ||
commit("setBlockMetaInfo", blockMetaInfo) | ||
) | ||
]) | ||
.then(() => { | ||
state.blockLoading = false | ||
dispatch("queryTxInfo", height) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you return this, I think you save yourself the inner .catch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. refactored all the promises to be more concise |
||
.then(blockTxInfo => { | ||
commit("setBlockTxInfo", blockTxInfo) | ||
resolve() | ||
}) | ||
.catch(error => { | ||
return reject(error) | ||
}) | ||
}) | ||
.catch(error => { | ||
state.blockLoading = false | ||
return reject(error) | ||
}) | ||
}) | ||
}, | ||
async queryBlock({ state, commit }, height) { | ||
return new Promise(resolve => { | ||
|
@@ -61,6 +79,60 @@ export default ({ commit, node }) => { | |
}) | ||
}) | ||
}, | ||
async queryTxInfo({ state, dispatch }, height) { | ||
let blockTxInfo = state.blockTxs.find( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not store them per height? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was following method used by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually just updating the blockchain one for sake of PR scope There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually i think both of these need to stay as arrays as they are used in for loops There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you point me to where they are used in loops? What I have found, there is always a selection by height happening when blockTxs appear in code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oops i totally misinterpreted the unit test error relating to |
||
b => b.length && b[0].height === height | ||
) | ||
if (blockTxInfo) { | ||
return blockTxInfo | ||
} | ||
try { | ||
blockTxInfo = await dispatch("getTxs", { | ||
key: 0, | ||
len: | ||
state.block && state.block.data && state.block.data.txs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the purpose of these chains of &&'s? Seems like it makes things harder to debug because if the format changes or data is missing, it will silently pass this call and have undefined behavior inside getTxs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was to allow the tests that had very minimal dummy blocks to pass. but you're right if the block is malformed it should throw an error. i've updated the other tests to use a well formed block dummy |
||
? state.block.data.txs.length | ||
: 0, | ||
txs: | ||
state.block && state.block.data && state.block.data.txs | ||
? state.block.data.txs.slice(0) | ||
: [] | ||
}) | ||
blockTxInfo && state.blockTxs.push(blockTxInfo) | ||
return blockTxInfo | ||
} catch (error) { | ||
return Promise.reject(error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't that the default for async functions rejecting on error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i was having trouble getting the error reporting come out correctly in the tests so went through and made each error very explicit until i found the culprit, i'll walk it back |
||
} | ||
}, | ||
getTxs({ state, commit, dispatch }, { key, len, txs }) { | ||
// this function is recursice promie used as an async loop in order to query all tx | ||
// found in a block. it's made similarly to queryBlockInfo only there | ||
// is more than one async call to make. the txstring is included but might not | ||
// actually be useful. etherscan.io includes something similar but it's seldom helpful | ||
return new Promise(async (resolve, reject) => { | ||
if (key >= len) return resolve(txs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment stating what this statement accomplishes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
let txstring = atob(txs[key]) | ||
let hash = await convertTx(txs[key]) | ||
node | ||
.txs(hash) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why mix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. refactored all the promises to be more concise |
||
.then(data => { | ||
data.string = txstring | ||
txs[key] = data | ||
dispatch("getTxs", { key: key + 1, len, txs }) | ||
.then(txs => { | ||
resolve(txs) | ||
}) | ||
.catch(reject) | ||
}) | ||
.catch(err => { | ||
commit("notifyError", { | ||
title: `Couldn't query block`, | ||
body: err.message | ||
}) | ||
reject(err) | ||
}) | ||
}) | ||
}, | ||
async queryBlockInfo({ state, commit }, height) { | ||
let blockMetaInfo = state.blockMetas.find(b => b.header.height === height) | ||
if (blockMetaInfo) { | ||
|
@@ -77,7 +149,7 @@ export default ({ commit, node }) => { | |
}) | ||
resolve(null) | ||
} else { | ||
resolve(data.block_metas[0]) | ||
resolve(data.block_metas.length ? data.block_metas[0] : null) | ||
} | ||
} | ||
) | ||
|
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.
@jolesbi this is what you were looking for, right?