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

feat: allow changing L1 synced height via admin RPC/CLI #1044

Merged
merged 20 commits into from
Sep 24, 2024

Conversation

colinlyguo
Copy link
Member

@colinlyguo colinlyguo commented Sep 17, 2024

1. Purpose or design rationale of this PR

For two typical errors in the logs:

  • "failed to get batch chunk ranges, empty chunk block ranges": revert to a "safe" height, the updates are idempotent.
  • "Unexpected queue index in SyncService expected=805,224 got=805,329": revert to a height that contains the last synced L1 message.

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • feat: A new feature

3. Deployment tag versioning

Has the version in params/version.go been updated?

  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change

@0xmountaintop
Copy link

0xmountaintop commented Sep 18, 2024

How about we check if the new height is lower than the previously stored height?
If the new height is higher, we won't allow it to overwrite the previous value.

@colinlyguo
Copy link
Member Author

colinlyguo commented Sep 18, 2024

41ab50d

yeah. it's a reasonable defense. added in 41ab50d.

omerfirmak
omerfirmak previously approved these changes Sep 18, 2024
internal/web3ext/web3ext.go Outdated Show resolved Hide resolved
eth/api.go Outdated Show resolved Hide resolved
@colinlyguo
Copy link
Member Author

colinlyguo commented Sep 18, 2024

How about we check if the new height is lower than the previously stored height? If the new height is higher, we won't allow it to overwrite the previous value.

I found this brings up another issue, if someone updates the height to a very small height (e.g. block 1, which is << the deployment height), the height cannot be increased through this rpc.

eth/api.go Outdated Show resolved Hide resolved
@colinlyguo
Copy link
Member Author

colinlyguo commented Sep 19, 2024

local test:
setL1MessageSyncedL1Height: success case.

> admin.setL1MessageSyncedL1Height(20785078);
null

INFO [09-19|14:12:25.368|eth/api.go:278]                                        Setting L1 message synced L1 height      height=20,785,078
INFO [09-19|14:12:25.369|rollup/sync_service/sync_service.go:155]               Reset sync service                       height=20,785,078
INFO [09-19|14:12:25.369|rollup/sync_service/sync_service.go:101]               Starting L1 message sync service         latestProcessedBlock=20,785,078
TRACE[09-19|14:12:26.332|rollup/sync_service/sync_service.go:173]               Sync service fetchMessages               latestProcessedBlock=20,785,078 latestConfirmed=20,785,074
TRACE[09-19|14:12:36.332|rollup/sync_service/sync_service.go:173]               Sync service fetchMessages               latestProcessedBlock=20,785,078 latestConfirmed=20,785,074

> scroll.syncStatus
{
  l1MessageSyncHeight: 20785106, -> a new height which > 20785078
  l1RollupSyncHeight: 18393700,
  l2BlockSyncHeight: 517679,
  l2FinalizedBlockHeight: 165610
}

setL1MessageSyncedL1Height: a wrong height.

DEBUG[09-19|14:09:35.352|rollup/sync_service/sync_service.go:241]               Received new L1 events                   fromBlock=20,670,001 toBlock=20,670,100 count=7
ERROR[09-19|14:09:35.353|rollup/sync_service/sync_service.go:252]               Unexpected queue index in SyncService    expected=937,164 got=932,518 msg="{QueueIndex:932518 Gas:168000 To:0x781e90f1c8Fc4611c9b7497C3B47F99Ef6969CbC Value:+0 Data:[142 241 51 46 0 0 0 0 0 0 0 0 0 0 0 0 83 163 156 144 242 176 244 166 135 92 196 20 104 205 235 5 64 68 146 215 0 0 0 0 0 0 0 0 0 0 0 0 83 163 156 144 242 176 244 166 135 92 196 20 104 205 235 5 64 68 146 215 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 11 212 158 11 26 32 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 14 58 166 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 160 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Sender:0x7885BcBd5CeCEf1336b5300fb5186A12DDD8c478}"

setRollupEventSyncedL1Height: success case.

> admin.setRollupEventSyncedL1Height(18306000);
null

INFO [09-19|13:50:15.749|eth/api.go:278]                                        Setting L1 message synced L1 height      height=18,306,000
INFO [09-19|13:50:15.750|rollup/sync_service/sync_service.go:155]               Reset sync service                       height=18,306,000
INFO [09-19|13:50:15.750|rollup/sync_service/sync_service.go:101]               Starting L1 message sync service         latestProcessedBlock=18,306,000
TRACE[09-19|13:50:16.693|rollup/sync_service/sync_service.go:173]               Sync service fetchMessages               latestProcessedBlock=18,306,000 latestConfirmed=20,784,978

> scroll.syncStatus
{
  l1MessageSyncHeight: 20784946,
  l1RollupSyncHeight: 18309600, -> a new height which > 18306000
  l2BlockSyncHeight: 508165,
  l2FinalizedBlockHeight: 443195
}

setRollupEventSyncedL1Height: a wrong height.

ERROR[09-19|13:41:21.751|rollup/rollup_sync_service/rollup_sync_service.go:206] failed to parse and update rollup event logs err="failed to get local node info, batch index: 8518, err: failed to get batch chunk ranges, empty chunk block ranges"

@colinlyguo colinlyguo changed the title feat: allow changing L1 synced height via RPC/CLI feat: allow changing L1 synced height via admin RPC/CLI Sep 19, 2024
@colinlyguo
Copy link
Member Author

colinlyguo commented Sep 24, 2024

tested again, an example:

Error logs:

2024-09-24 23:05:47 ERROR[09-24|15:05:47.092] Unexpected queue index in SyncService    expected=937,164 got=937,167 msg="{QueueIndex:937167 Gas:168000 To:0x781e90f1c8Fc4611c9b7497C3B47F99Ef6969CbC Value:+0 Data:[142 241 51 46 0 0 0 0 0 0 0 0 0 0 0 0 199 173 131 93 218 166 136 142 44 78 108 65 49 167 133 233 36 249 188 67 0 0 0 0 0 0 0 0 0 0 0 0 199 173 131 93 218 166 136 142 44 78 108 65 49 167 133 233 36 249 188 67 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 111 69 0 55 230 120 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 14 76 207 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 160 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Sender:0x7885BcBd5CeCEf1336b5300fb5186A12DDD8c478}"

get fixed height by:

select l1_block_number from cross_message_v2 where message_nonce = 937163;
l1_block_number|
---------------+
       20784971|

call admin API:

> admin.setL1MessageSyncedL1Height(20784971);
null

recovered:

2024-09-24 23:07:26 INFO [09-24|15:07:26.933] Syncing L1 messages                      processed=20,799,371 confirmed=20,821,179 collected=425 progress(%)=99.895
2024-09-24 23:07:34 INFO [09-24|15:07:34.426] Imported new chain segment               blocks=37  txs=357 mgas=38.265 elapsed=8.073s    mgasps=4.740 number=655,389 hash=181cca..9c3a8f age=10mo2w6d dirty=0.00B
2024-09-24 23:07:36 INFO [09-24|15:07:36.845] Syncing L1 messages                      processed=20,802,371 confirmed=20,821,179 collected=498 progress(%)=99.910

if changing to a wrong height, will show:

2024-09-24 23:06:27 ERROR[09-24|15:06:27.101] Unexpected queue index in SyncService    expected=937,164 got=937,165 msg="{QueueIndex:937165 Gas:150000 To:0x781e90f1c8Fc4611c9b7497C3B47F99Ef6969CbC Value:+0 Data:[142 241 51 46 0 0 0 0 0 0 0 0 0 0 0 0 91 207 217 156 52 207 126 6 252 117 111 111 90 231 64 5 4 133 43 196 0 0 0 0 0 0 0 0 0 0 0 0 161 161 33 88 190 98 105 215 88 12 99 236 94 96 156 220 13 221 130 188 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 11 38 107 178 251 127 128 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 14 76 205 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 160 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Sender:0x7885BcBd5CeCEf1336b5300fb5186A12DDD8c478}"

@colinlyguo colinlyguo merged commit cf1ca84 into develop Sep 24, 2024
8 checks passed
@colinlyguo colinlyguo deleted the feat-allow-changing-l1-synced-height branch September 24, 2024 19:18
0xmountaintop pushed a commit that referenced this pull request Oct 10, 2024
* feat: allow changing L1 synced height via RPC/CLI

* chore: auto version bump [bot]

* fix typos

* add height validity check

* change implementation

* moved hotfix apis from ScrollAPI to PrivateAdminAPI

* change namespace from scroll to admin

* fix bugs

* add locks

* add a lock to protect latestProcessedBlock and db state

* revert some changes

* remove lock and use atomic in latestProcessedBlock

* fix CI

* tweak

* use cas in updating latestProcessedBlock

* renaming

* also update sync height in error paths

* fix a bug

* use mutex lock

---------

Co-authored-by: colinlyguo <[email protected]>
0xmountaintop added a commit that referenced this pull request Oct 10, 2024
* feat: allow changing L1 synced height via admin RPC/CLI (#1044)

* feat: allow changing L1 synced height via RPC/CLI

* chore: auto version bump [bot]

* fix typos

* add height validity check

* change implementation

* moved hotfix apis from ScrollAPI to PrivateAdminAPI

* change namespace from scroll to admin

* fix bugs

* add locks

* add a lock to protect latestProcessedBlock and db state

* revert some changes

* remove lock and use atomic in latestProcessedBlock

* fix CI

* tweak

* use cas in updating latestProcessedBlock

* renaming

* also update sync height in error paths

* fix a bug

* use mutex lock

---------

Co-authored-by: colinlyguo <[email protected]>

* more

---------

Co-authored-by: colin <[email protected]>
Co-authored-by: colinlyguo <[email protected]>
lwedge99 pushed a commit to sentioxyz/scroll-geth that referenced this pull request Dec 16, 2024
…1044)

* feat: allow changing L1 synced height via RPC/CLI

* chore: auto version bump [bot]

* fix typos

* add height validity check

* change implementation

* moved hotfix apis from ScrollAPI to PrivateAdminAPI

* change namespace from scroll to admin

* fix bugs

* add locks

* add a lock to protect latestProcessedBlock and db state

* revert some changes

* remove lock and use atomic in latestProcessedBlock

* fix CI

* tweak

* use cas in updating latestProcessedBlock

* renaming

* also update sync height in error paths

* fix a bug

* use mutex lock

---------

Co-authored-by: colinlyguo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants