-
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
F 51 refactor event processing #54
Conversation
@aminlatifi I had problem on installing dependencies, so I updated giveth bridge dependencies as well |
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.
Please update the event index to facilitate the sorting and finding the first event to process. Current index is
{ isHomeEvent: 1, blockNumber: 1, transactionIndex: 1, logIndex: 1 },
{ | ||
hash: { type: String, required: true, index: true }, | ||
from: { type: String }, | ||
gasPrice: { type: Number }, |
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.
Just fields of hash
, blockNumber
, isHome
and timestamp
are needed, plus transactionIndex
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.
You are right, extra fields removed
@aminlatifi
feathers-src/lib/populate.js
Outdated
@@ -38,6 +38,9 @@ const processNextWaitingEvent = async () => { | |||
const query = { | |||
status: EventStatus.WAITING, | |||
$sort: { | |||
// maybe we get some events from home and blockchain with different blockNumber, so the transactionTime | |||
// should be the first important thing for decide what event should process first | |||
timestamp: 1, | |||
isHomeEvent: 1, |
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.
Maybe this one can be removed
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.
isHomeEvent
removed
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.
Event blockNumber I guess is not needed
related #51