Skip to content

[Clarity-4] Implement block-time #6478

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Sep 16, 2025
Merged

[Clarity-4] Implement block-time #6478

merged 16 commits into from
Sep 16, 2025

Conversation

@obycode
Copy link
Contributor

@obycode obycode commented Sep 11, 2025

Description

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo

@obycode
feat: initial setup for block-time keyword
dda4af1
@obycode
feat: implement block-time
2b9e955 
This new keyword retrieves the timestamp for the current block by
storing it in the MARF. This means it also works as expected inside of
an `at-block` context.
@obycode obycode requested review from a team as code owners September 11, 2025 12:41
@obycode obycode moved this to Status: In Review in Stacks Core Eng Sep 11, 2025
@obycode obycode added this to the 3.3.0.0.0 milestone Sep 11, 2025
@obycode obycode linked an issue Sep 11, 2025 that may be closed by this pull request
[Clarity-4] block-time #6423
Closed
@obycode
Copy link
Contributor Author

obycode commented Sep 11, 2025

Seems I still have some issues with the MARF in the unit tests. I'm taking a look.

jferrant
jferrant reviewed Sep 11, 2025
View reviewed changes
jferrant
jferrant reviewed Sep 11, 2025
View reviewed changes
@obycode
Copy link
Contributor Author

obycode commented Sep 11, 2025

Note to reviewers: I am definitely open to ideas for a better way to do this without passing the block time around so much.

@obycode
Copy link
Contributor Author

obycode commented Sep 11, 2025

Also, I'm realizing that I failed to epoch gate the setting of the timestamp in the MARF. That definitely needs to be fixed.

@obycode obycode marked this pull request as draft September 11, 2025 20:42
@obycode obycode marked this pull request as ready for review September 12, 2025 15:12
@obycode obycode requested a review from kantai September 12, 2025 15:15
@obycode
Copy link
Contributor Author

obycode commented Sep 12, 2025

Issues should be fixed and ready for review now.

jferrant
jferrant reviewed Sep 12, 2025
View reviewed changes
@obycode obycode requested a review from jferrant September 15, 2025 13:06
kantai
kantai reviewed Sep 15, 2025
View reviewed changes
Copy link
Contributor

@kantai kantai left a comment

Choose a reason for hiding this comment

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

I think we should take a bit of a different approach on how we insert this data into the MARF. I think it should be possible to make this data a little more abstracted from the MARF internals, which should simplify this PR.

@obycode obycode requested a review from kantai September 16, 2025 00:06
@obycode obycode marked this pull request as draft September 16, 2025 13:39
@obycode
Copy link
Contributor Author

obycode commented Sep 16, 2025

Converted to draft while fixing conflicts and checking tests.

@obycode obycode marked this pull request as ready for review September 16, 2025 13:51
@obycode
Copy link
Contributor Author

obycode commented Sep 16, 2025

Should be good to go.

jferrant
jferrant reviewed Sep 16, 2025
View reviewed changes
jferrant
jferrant previously approved these changes Sep 16, 2025
View reviewed changes
Copy link
Contributor

@jferrant jferrant left a comment

Choose a reason for hiding this comment

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

Honestly just ignore my Nit :P LGTM!

kantai
kantai reviewed Sep 16, 2025
View reviewed changes
kantai
kantai reviewed Sep 16, 2025
View reviewed changes
Copy link
Contributor

@kantai kantai left a comment

Choose a reason for hiding this comment

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

LGTM, just had one comment on possibly unused From impls

@obycode obycode requested review from jferrant and kantai September 16, 2025 17:40
@obycode obycode enabled auto-merge September 16, 2025 18:25
@obycode obycode added this pull request to the merge queue Sep 16, 2025
Merged via the queue into stacks-network:develop with commit 307d088 Sep 16, 2025
1 of 2 checks passed
@obycode obycode deleted the feat/block-time branch September 16, 2025 19:02
@github-project-automation github-project-automation bot moved this from Status: In Review to Status: ✅ Done in Stacks Core Eng Sep 16, 2025
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@kantai kantai kantai approved these changes

@jferrant jferrant jferrant approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

locked

Projects

Stacks Core Eng
Status: Status: ✅ Done

Milestone

3.3.0.0.0

Development

Successfully merging this pull request may close these issues.

[Clarity-4] block-time

3 participants

@obycode @kantai @jferrant