Skip to content

Comments

feat(context): add mark_cold method to JournaledAccount#3160

Merged
rakita merged 2 commits intobluealloy:mainfrom
celo-org:karlb/add-mark_cold
Nov 12, 2025
Merged

feat(context): add mark_cold method to JournaledAccount#3160
rakita merged 2 commits intobluealloy:mainfrom
celo-org:karlb/add-mark_cold

Conversation

@karlb
Copy link
Contributor

@karlb karlb commented Nov 11, 2025

Add a public mark_cold() method to JournaledAccount that delegates to the underlying Account::mark_cold(). This is analogous to the other methods and provides a way to mark accounts as cold that was available in previous revm versions.

This is used by celo-revm to provide backwards compatibility for the transfer precompile, which does not warm addresses in its initial implementation.
See https://github.com/celo-org/celo-kona/blob/f466c56a909513dbdc4b2fbfa4445121aaaea82d/crates/celo-revm/src/precompiles/transfer.rs#L134-L141 and https://github.com/celo-org/celo-kona/blob/f466c56a909513dbdc4b2fbfa4445121aaaea82d/crates/celo-revm/src/precompiles/transfer.rs#L107-L109

Add a public `mark_cold()` method to `JournaledAccount` that delegates
to the underlying `Account::mark_cold()`. This is analogous and provides
a way to mark accounts as cold that was available in previous revm
versions.

This is used by celo-revm to provide backwards compatibility for the
transfer precompile, which does not warm addresses in its initial
implementation.
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 11, 2025

CodSpeed Performance Report

Merging #3160 will not alter performance

Comparing celo-org:karlb/add-mark_cold (c5ae7d2) with main (5723170)

Summary

✅ 173 untouched

@karlb karlb marked this pull request as ready for review November 11, 2025 14:03
@rakita
Copy link
Member

rakita commented Nov 12, 2025

Making a change in Account without journal entry can be a footgun if call gets reverted. But for celo use case making account warm and than marking it cold in same call negates this.

Will rename it as unsafe_mark_cold() and add comment to explain why it is unsafe

and add explanation.

Co-authored-by: rakita <rakita@users.noreply.github.com>
@rakita rakita merged commit 9843e21 into bluealloy:main Nov 12, 2025
31 checks passed
@github-actions github-actions bot mentioned this pull request Nov 12, 2025
@karlb
Copy link
Contributor Author

karlb commented Nov 12, 2025

Thanks, this helps us to avoid one more fork!

@karlb karlb deleted the karlb/add-mark_cold branch November 12, 2025 10:34
karlb added a commit to celo-org/celo-kona that referenced this pull request Nov 12, 2025
This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.
karlb added a commit to celo-org/celo-kona that referenced this pull request Nov 12, 2025
This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.
seolaoh pushed a commit to celo-org/celo-kona that referenced this pull request Nov 12, 2025
This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.
karlb added a commit to celo-org/celo-kona that referenced this pull request Nov 12, 2025
This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.
karlb added a commit to celo-org/celo-kona that referenced this pull request Nov 17, 2025
* unsafe: Avoid account.mark_cold

This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.

* Implement CIP-64 system calls without state merging

Keep all state changes in the EVM journal instead of using set_storage:
- System calls execute without calling finalize()
- State changes remain in journal for main transaction to see
- Add call_read_only wrapper using checkpoint/revert to prevent accidental state changes in query operations
- Apply read-only wrapper to get_balance, get_currencies, get_exchange_rates, get_intrinsic_gas

This eliminates the need for manual state merging and the revm fork.

* Stop using Celo revm fork

Now that we use neither `Account.mark_cold` nor `set_storage`, we can go
back to using upstream revm.

* Add execution test

* Remove double journaled

* Test for revert deposit

* Remove double set_balance

* Prune to be closer to upstream

* Fix nits

* Bump hokulea to v1.0.2

* Docstring changes after PR discussion

This makes things at least a bit clearer. I'm having a hard to adding
explanations for all important aspects without creating a lot of text
and linking to constantly changing revm code.

---------

Co-authored-by: Gastón Ponti <gaston.ponti@clabs.co>
Co-authored-by: seolaoh <osa8361@gmail.com>
karlb added a commit to celo-org/celo-kona that referenced this pull request Nov 17, 2025
This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.
karlb added a commit to celo-org/celo-kona that referenced this pull request Dec 8, 2025
* Bump kona to v1.1.7 and revm to v31.0.0 (#99)

* Initial bump to kona v1.1.7

* Replace revm crates with custom fork which has two new func

* Resolved version revm conflict between crates-io and github fork

* Fix compile errors

* Fix tests

* Fix test_transfer

* Fix handler tests

* Fork revm in celo-org

* Fix lint errors

* Patch revm with our forked version of v97 (#100)

This ensures that we are using the same revm version as in our
op-succinct repo. The v97 version fixed an execution bug that would have
led to an execution mismatch between the op-stack and our
succinct-stack.

Also updates dependencies to point at the versions exported by the base
of our forked revm.

* Update hokulea to v1.0.1 (#101)

This version is required for L2Beat to put us in the 'validium &
optimimum' category.

* unsafe: Avoid account.mark_cold

This is just a temporary measure until we switch to an revm version
including bluealloy/revm#3160.

* Implement CIP-64 system calls without state merging

Keep all state changes in the EVM journal instead of using set_storage:
- System calls execute without calling finalize()
- State changes remain in journal for main transaction to see
- Add call_read_only wrapper using checkpoint/revert to prevent accidental state changes in query operations
- Apply read-only wrapper to get_balance, get_currencies, get_exchange_rates, get_intrinsic_gas

This eliminates the need for manual state merging and the revm fork.

* Stop using Celo revm fork

Now that we use neither `Account.mark_cold` nor `set_storage`, we can go
back to using upstream revm.

* Add execution test

* Remove double journaled

* Test for revert deposit

* Remove double set_balance

* Prune to be closer to upstream

* Fix nits

* Bump hokulea to v1.0.2

* Docstring changes after PR discussion

This makes things at least a bit clearer. I'm having a hard to adding
explanations for all important aspects without creating a lot of text
and linking to constantly changing revm code.

---------

Co-authored-by: Seola Oh <osa8361@gmail.com>
Co-authored-by: piersy <pierspowlesland@gmail.com>
Co-authored-by: Gastón Ponti <gaston.ponti@clabs.co>
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.

2 participants