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

Ntime for jobs #12

Merged
merged 6 commits into from Oct 25, 2022
Merged

Ntime for jobs #12

merged 6 commits into from Oct 25, 2022

Conversation

jakubtrnka
Copy link
Collaborator

No description provided.

@jakubtrnka jakubtrnka force-pushed the ntime-for-jobs branch 3 times, most recently from d27e410 to c8f197f Compare October 4, 2022 15:33
+----------------+---------------+-------------------------------------------------------------------------------------+
| job_id | U32 | Identifier of the job as provided by NewMiningJob or NewExtendedMiningJob message |
+----------------+---------------+-------------------------------------------------------------------------------------+
| starting_ntime | OPTION[u32] | Empty if the job is intended for a future SetNewPrevHash message sent on this |
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the name min_ntime, but if you don't like that for any reason, starting_ntime is also fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was @TheBlueMatt suggestion. I don't have strong opinion on that.

Choose a reason for hiding this comment

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

I thought the discord discussion ended up with min, I could be wrong.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Too many communication channels, because I don't remember talking about this on discord. But whatever, let's keep the the original min_ntime.

Copy link
Contributor

@rrybarczyk rrybarczyk left a comment

Choose a reason for hiding this comment

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

Overall looks good!

@jakubtrnka
Copy link
Collaborator Author

Btw I took the liberty and removed the confusing label <Bitcoin specific part> from the new_mining_job message.
It served nor real purpose.

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.

None yet

3 participants