Conversation
saratnt
requested review from
MikelHorizenLabs,
alsala,
andreanistico and
paolocappelletti
docs/design/BATCH_EXECUTION.md
Outdated
|
|
||
| // WASM execution — can cause soft failure | ||
| appState := appData.GetAppState() | ||
| run WASM (Deposit if needed, then ProcessRequest) |
There was a problem hiding this comment.
Maybe we should explicitly add that appData is NOT mutated by a deposit until the full request (deposit + process) succeeds
docs/design/BATCH_EXECUTION.md
Outdated
| `GetPendingRequestsWithStateRoot` adds an `applicationId` parameter: | ||
|
|
||
| ``` | ||
| GetPendingRequestsWithStateRoot(applicationId uint64, maxCount uint64) ([]*common.Request, [32]byte, error) |
There was a problem hiding this comment.
proposal: we could move in the contract the logic to loop through the applications' queues:
- the contract knows the list of applicationId deployed, so can loop on every GetPendingRequestsWithStateRoot call
- the applicationId is in the returned data of GetPendingRequestsWithStateRoot: every time it is called it returns the next non-empty app request list, in a loop style (ordered by deploy time) - method signature becomes:
GetPendingRequestsWithStateRoot(maxCount uint64) (applicationId uint64, []*common.Request, [32]byte, error)
Pro: prevent manager censorship
what do you think?
There was a problem hiding this comment.
I think it may be a good idea. GetPendingRequestsWithStateRoot could use as algorithm to return the batch from the app with the oldest first request in the queue. This means we could have 2 or more consecutive batches from the same app but at least it's fair and everybody can verify its fainess. What do you think?
docs/design/BATCH_EXECUTION.md
Outdated
| - **Single stateRoot storage write**: read `stateRoot` once at the start, chain through entries in memory, write once at the end — saves (N-1) warm `SSTORE` operations (~5,000 gas each) | ||
| - **State root chain validation**: only the first entry checks `prevStateRoot == stateRoot` from storage; subsequent entries validate `entries[i].prevStateRoot == entries[i-1].newStateRoot` in memory | ||
| - **Deduplicated `applicationId`**: passed once instead of per-entry | ||
| - **Optional batch signature**: verify one signature over the full batch instead of N individual `ecrecover` calls, saving (N-1) × ~3,000+ gas (individual per-request signatures can still be emitted in events for off-chain verifiability) |
There was a problem hiding this comment.
i would implement immediately the batch signature - can save gas and computation time, and does not look to me too complex
4 participants
Jira: https://horizenlabs.atlassian.net/browse/HZN-2783?atlOrigin=eyJpIjoiZDUzNGI0MjIyYTRhNDFlZmE1MGIwNmI2MmVjZjAzYzQiLCJwIjoiaiJ9