feat(contracts): add update_start_time with on-chain emission and backend sync (#589) - #656
Open
goodness-cpu wants to merge 1 commit into
Open
feat(contracts): add update_start_time with on-chain emission and backend sync (#589)#656goodness-cpu wants to merge 1 commit into
goodness-cpu wants to merge 1 commit into
Conversation
…kend sync (ritik4ever#589) Implements the missing on-chain counterpart of the existing backend start_time_updated event history type. contract (contracts/src/lib.rs): * Add StreamStartTimeUpdated event struct with stream_id, sender, old_start_time, new_start_time, and updated_at fields. * Add update_start_time(stream_id, sender, new_start_time) function: - Sender-only auth (require_auth + sender-mismatch panic, matching pause/resume/cancel). - Rejects canceled streams. - Validates new_start_time > current ledger time (new_start_time must be in the future). - Validates new_start_time < stream.end_time. - Updates storage and emits Stream/StartUp event so off-chain listeners can reconcile. backend (backend/src/services/indexer.ts): * Add StartUp case to processEvent that records a start_time_updated history row with oldStartTime and newStartTime metadata, mirroring the resume/pause pattern. tests (contracts/src/test.rs): * test_update_start_time_success: scheduled stream update persists to storage. * test_update_start_time_emits_event: asserts Stream+StartUp topic and full StreamStartTimeUpdated payload. * test_update_start_time_fails_with_wrong_sender: sender mismatch panics. * test_update_start_time_rejects_past_timestamp: past-or-equal time panics. * test_update_start_time_rejects_at_end_time: new_start_time == end_time panics. * test_update_start_time_rejects_after_end_time: new_start_time > end_time panics. * test_update_start_time_rejects_on_canceled_stream: canceled stream panics. Acceptance criteria: * Event type start_time_updated is recorded in DB once the on-chain call lands and the indexer processes the new Stream/StartUp event. * Frontend StreamTimeline already renders start_time_updated events with the clock icon and label. closes ritik4ever#589
|
@goodness-cpu is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@goodness-cpu Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughAdds a contract method to update a stream’s start time with validation and event emission, extends contract tests for success and failure cases, and updates the backend indexer to persist the corresponding history event. ChangesStream start time updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant update_start_time
participant processEvent
participant recordEventWithDb
update_start_time->>processEvent: Emits Stream/StartUp
processEvent->>recordEventWithDb: Records start_time_updated with oldStartTime and newStartTime
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
contracts/src/test.rs (2)
2402-2421: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExercise authentication separately from sender matching.
Because
env.mock_all_auths()authorizes every address and the contract checkssender mismatchbeforesender.require_auth(), this test would still pass if the authorization check were removed. Add a test using the Soroban auth-mocking API that supplies the legitimate sender without granting authorization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/src/test.rs` around lines 2402 - 2421, The test_update_start_time_fails_with_wrong_sender test currently cannot verify authentication because mock_all_auths authorizes every address. Replace that setup with Soroban auth mocking that authorizes only the legitimate sender, while still invoking update_start_time with wrong_sender; preserve the expected sender mismatch panic and ensure the test would fail if sender.require_auth() were removed.
2423-2442: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the exact
new_start_time == nowboundary.The implementation rejects
new_start_time <= now, but this test only covers a timestamp in the past. Add a case where both values equal2000so a regression from<=to<is detected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/src/test.rs` around lines 2423 - 2442, The test test_update_start_time_rejects_past_timestamp only covers a timestamp earlier than the current ledger time. Add a separate boundary test, or adjust the setup, so env.ledger().timestamp and the update_start_time argument both equal 2000, while retaining the expected “new_start_time must be in the future” panic.backend/src/services/indexer.ts (1)
300-315: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an indexer regression test for
StartUp.Add coverage in
backend/src/services/indexer.test.tsasserting that the event produces astart_time_updatedrow with the sender, ledger sequence, and both old/new metadata fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/services/indexer.ts` around lines 300 - 315, Add a regression test in indexer.test.ts for the StartUp handling path, invoking the indexer with representative startup data and asserting that a start_time_updated row is created with the expected sender, ledger sequence, oldStartTime, and newStartTime values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/services/indexer.ts`:
- Around line 300-315: Update the StartUp handling in recordEventWithDb so
value.old_start_time and value.new_start_time are preserved exactly instead of
being coerced with Number(...). Store them as decimal strings or the
repository’s established exact u64 representation, and update any affected
consumers or callers to handle that representation without precision loss.
---
Nitpick comments:
In `@backend/src/services/indexer.ts`:
- Around line 300-315: Add a regression test in indexer.test.ts for the StartUp
handling path, invoking the indexer with representative startup data and
asserting that a start_time_updated row is created with the expected sender,
ledger sequence, oldStartTime, and newStartTime values.
In `@contracts/src/test.rs`:
- Around line 2402-2421: The test_update_start_time_fails_with_wrong_sender test
currently cannot verify authentication because mock_all_auths authorizes every
address. Replace that setup with Soroban auth mocking that authorizes only the
legitimate sender, while still invoking update_start_time with wrong_sender;
preserve the expected sender mismatch panic and ensure the test would fail if
sender.require_auth() were removed.
- Around line 2423-2442: The test test_update_start_time_rejects_past_timestamp
only covers a timestamp earlier than the current ledger time. Add a separate
boundary test, or adjust the setup, so env.ledger().timestamp and the
update_start_time argument both equal 2000, while retaining the expected
“new_start_time must be in the future” panic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5da8aa07-0693-4db2-8d93-69fe5f21e104
📒 Files selected for processing (3)
backend/src/services/indexer.tscontracts/src/lib.rscontracts/src/test.rs
Comment on lines
+300
to
+315
|
|
||
| case "StartUp": | ||
| recordEventWithDb( | ||
| db, | ||
| value.stream_id.toString(), | ||
| "start_time_updated", | ||
| timestamp, | ||
| value.sender, | ||
| undefined, | ||
| { | ||
| oldStartTime: Number(value.old_start_time), | ||
| newStartTime: Number(value.new_start_time), | ||
| }, | ||
| event.ledger, | ||
| ); | ||
| break; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate indexer and schemas =="
fd -a 'indexer\.ts|schemas\.ts$' . | sed 's#^./##'
echo "== relevant indexer section =="
idx="$(fd 'indexer\.ts$' . | head -n1)"
if [ -n "${idx:-}" ]; then
wc -l "$idx"
sed -n '1,70p;230,340p' "$idx" | cat -n
fi
echo "== event record implementation =="
rg -n "recordEventWithDb|function recordEventWithDb|record.*Event|event.*metadata|JSON\.stringify|oldStartTime|newStartTime|start_time_updated" -S .
echo "== probe u64 Number.MAX_SAFE_INTEGER issue =="
node - <<'JS'
const values = [
BigInt(String(Number.MAX_SAFE_INTEGER)),
BigInt(String(Number.MAX_SAFE_INTEGER + 1)),
18446744073709551615n
];
for (const value of values) {
const num = Number(value);
const back = BigInt.asIntN(64, BigInt(num));
console.log(JSON.stringify({
bigint: value.toString(),
number: num,
backToBigInt: back.toString(),
exact: value.toString() === back.toString(),
aboveSafe: value > BigInt(Number.MAX_SAFE_INTEGER),
}));
}
JSRepository: ritik4ever/stellar-stream
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -a 'indexer\.ts$' .
fd -a 'schemas\.ts$' .
echo "== target file lines =="
idx="$(fd 'indexer\.ts$' . | head -n1)"
if [ -n "${idx:-}" ]; then
echo "INDEXER=$idx"
wc -l "$idx"
sed -n '1,90p;240,330p' "$idx" | nl -ba
fi
echo "== focused event/event metadata functions and schema usages =="
rg -n "recordEventWithDb|function recordEventWithDb|const recordEventWithDb|StartUp|start_time_updated|oldStartTime|newStartTime|value\.old_start_time|value\.new_start_time|JSON\.stringify|RecordEvent" -S "$idx" $(fd 'schemas\.ts$' . | head -n5 || true)
echo "== Node u64 Number precision probe =="
node - <<'JS'
const values = [
BigInt(String(Number.MAX_SAFE_INTEGER)),
BigInt(String(Number.MAX_SAFE_INTEGER) + "+1"),
18446744073709551615n
];
for (const value of values) {
const num = Number(value);
const back = BigInt(num) & ((1n << 64n) - 1n);
console.log(JSON.stringify({
bigint: value.toString(),
number: num,
backThroughBigIntNumber: back.toString(),
exact: value.toString() === back.toString(),
aboveSafe: value > BigInt(Number.MAX_SAFE_INTEGER),
}));
}
JSRepository: ritik4ever/stellar-stream
Length of output: 488
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file lines =="
sed -n '1,90p;240,330pM' backend/src/services/indexer.ts | awk '{printf "%6d\t%s\n", NR+0, $0}'
echo "== focused event/event metadata functions and schema usages =="
rg -n "recordEventWithDb|function recordEventWithDb|const recordEventWithDb|StartUp|start_time_updated|oldStartTime|newStartTime|value\.old_start_time|value\.new_start_time|JSON\.stringify|RecordEvent" -S backend/src/services/indexer.ts backend/src/validation/schemas.ts
echo "== Node u64 Number precision probe =="
node - <<'JS'
const values = [
BigInt(String(Number.MAX_SAFE_INTEGER)),
BigInt(String(Number.MAX_SAFE_INTEGER) + "+1"),
18446744073709551615n
];
for (const value of values) {
const num = Number(value);
const back = BigInt(num) & ((1n << 64n) - 1n);
console.log(JSON.stringify({
bigint: value.toString(),
number: num,
backThroughBigIntNumber: back.toString(),
exact: value.toString() === back.toString(),
aboveSafe: value > BigInt(Number.MAX_SAFE_INTEGER),
}));
}
JSRepository: ritik4ever/stellar-stream
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file lines =="
sed -n '1,90p;240,330p' backend/src/services/indexer.ts | awk '{printf "%6d\t%s\n", NR+0, $0}'
echo "== focused event/event metadata functions and schema usages =="
rg -n "recordEventWithDb|function recordEventWithDb|const recordEventWithDb|StartUp|start_time_updated|oldStartTime|newStartTime|value\.old_start_time|value\.new_start_time|JSON\.stringify|RecordEvent" -S backend/src/services/indexer.ts backend/src/validation/schemas.ts
echo "== Node u64 Number precision probe =="
node - <<'JS'
const values = [
BigInt(String(Number.MAX_SAFE_INTEGER)),
BigInt(String(Number.MAX_SAFE_INTEGER) + "+1"),
18446744073709551615n
];
for (const value of values) {
const num = Number(value);
const back = BigInt(num) & ((1n << 64n) - 1n);
console.log(JSON.stringify({
bigint: value.toString(),
number: num,
backThroughBigIntNumber: back.toString(),
exact: value.toString() === back.toString(),
aboveSafe: value > BigInt(Number.MAX_SAFE_INTEGER),
}));
}
JSRepository: ritik4ever/stellar-stream
Length of output: 7429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== eventHistory related files =="
fd -a 'eventHistory\.ts$' backend/src/services
echo "== eventHistory implementation =="
evt="$(fd 'eventHistory\.ts$' backend/src/services | head -n1)"
if [ -n "${evt:-}" ]; then
wc -l "$evt"
sed -n '1,220p' "$evt" | awk '{printf "%6d\t%s\n", NR+0, $0}'
fi
echo "== backend validation schema event rows =="
rg -n "event_history|start_time_updated|oldStartTime|newStartTime|JSON|metadata" -S backend/src backend/src/services backend/src/validation || true
echo "== Node u64 Number precision probe =="
node - <<'JS'
const values = [
BigInt(String(Number.MAX_SAFE_INTEGER)),
BigInt(number = Number.MAX_SAFE_INTEGER + 1),
18446744073709551615n
];
for (const value of values) {
const num = Number(value);
const back = BigInt(num) & ((1n << 64n) - 1n);
console.log(JSON.stringify({
bigint: value.toString(),
number: num,
backThroughBigIntNumber: back.toString(),
exact: value.toString() === back.toString(),
aboveSafe: value > BigInt(Number.MAX_SAFE_INTEGER),
}));
}
JSRepository: ritik4ever/stellar-stream
Length of output: 27723
Do not coerce on-chain u64 timestamps to Number.
Valid timestamps above Number.MAX_SAFE_INTEGER lose precision when passed through Number(...) before JSON.stringify, so oldStartTime/newStartTime can be corrupted in persisted event metadata. Store these values as decimal strings or another exact representation and update callers accordingly.
Preserve exact timestamp values
- oldStartTime: Number(value.old_start_time),
- newStartTime: Number(value.new_start_time),
+ oldStartTime: String(value.old_start_time),
+ newStartTime: String(value.new_start_time),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case "StartUp": | |
| recordEventWithDb( | |
| db, | |
| value.stream_id.toString(), | |
| "start_time_updated", | |
| timestamp, | |
| value.sender, | |
| undefined, | |
| { | |
| oldStartTime: Number(value.old_start_time), | |
| newStartTime: Number(value.new_start_time), | |
| }, | |
| event.ledger, | |
| ); | |
| break; | |
| case "StartUp": | |
| recordEventWithDb( | |
| db, | |
| value.stream_id.toString(), | |
| "start_time_updated", | |
| timestamp, | |
| value.sender, | |
| undefined, | |
| { | |
| oldStartTime: String(value.old_start_time), | |
| newStartTime: String(value.new_start_time), | |
| }, | |
| event.ledger, | |
| ); | |
| break; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/services/indexer.ts` around lines 300 - 315, Update the StartUp
handling in recordEventWithDb so value.old_start_time and value.new_start_time
are preserved exactly instead of being coerced with Number(...). Store them as
decimal strings or the repository’s established exact u64 representation, and
update any affected consumers or callers to handle that representation without
precision loss.
Owner
|
Hi @goodness-cpu, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(contracts): add update_start_time with on-chain emission and backend sync
Closes #589
Summary
Implements the missing on-chain counterpart of the existing
start_time_updatedevent-history type in the backend. The backend alreadyrecorded this event type, but the contract function that should emit it was
never implemented. This PR adds:
update_start_time(stream_id, sender, new_start_time)— a newcontract function with sender-only auth.
new_start_timeto bestrictly in the future (relative to the ledger timestamp) and strictly less
than
stream.end_time(matching the issue's acceptance criteria).StreamStartTimeUpdatedevent — emitted under the existing(Stream, StartUp)topic so off-chain listeners (the Soroban eventindexer) can reconcile the change.
stream_eventswitholdStartTimeandnewStartTimemetadata so it surfaces in theStreamTimelineUI.Project areas
contracts/src/lib.rs— new event struct and function.backend/src/services/indexer.ts— newStartUpcase.contracts/src/test.rs— 7 new contract tests covering happy path,event payload, sender mismatch, past time, equal-to-end, after-end, and
canceled stream.
Acceptance criteria
start_time_updatedrecorded in DB after on-chain call.The indexer case
StartUpcallsrecordEventWithDb(... "start_time_updated" ...)usingINSERT OR IGNORE, which keeps it idempotent across replays.frontend/src/components/StreamTimeline.tsx:FILTER_BUTTONSincludesstart_time_updatedwith the 🕐 icon andthe Start Time Updated label.
getEventIcon,formatEventTitle, andgetEventDescriptionallhandle the event type.
eventHistory.tsStreamEventTypeunion already lists the type, so nobackend type changes were needed.
Contract changes (
contracts/src/lib.rs)Backend changes (
backend/src/services/indexer.ts)Tests added (
contracts/src/test.rs)test_update_start_time_success— scheduled stream update persists.test_update_start_time_emits_event—Stream/StartUptopic with fullpayload (old_start_time, new_start_time, updated_at).
test_update_start_time_fails_with_wrong_sender— sender mismatch panic.test_update_start_time_rejects_past_timestamp—new_start_time <= nowpanic.
test_update_start_time_rejects_at_end_time—new_start_time == end_timepanic.test_update_start_time_rejects_after_end_time—new_start_time > end_timepanic.test_update_start_time_rejects_on_canceled_stream— canceled streampanic.
Test run in this PR
streamStore.ts(
elapsed/ratioredeclaration incalculateProgress),webhooks*.test.ts(missing
stream_eventstable) and a staledurationSeconds=1validationexpectation were NOT introduced by this PR. Targeted runs of the relevant
suites pass:
src/services/eventHistory.test.ts— 11 passed.src/services/indexer.circuitbreaker.test.ts— 5 passed.src/services/streamStore.updateStartAt.test.tshas a pre-existingduplicate
cacheMocksdeclaration in the test file itself; fixing thatin the same area of the repo is recommended but outside the scope of
this issue.
Known follow-ups (not blocking)
updateStreamStartAtinbackend/src/services/streamStore.tsdoes a local DB write only and does not submit a Soroban transaction
invoking the new contract function (unlike
createStream/cancelStreamwhich do). Wiring that is a small backend-only follow-up — the contract
contract is now ready to receive such calls.
StartUp(e.g.UpdStart) in a future refactor; this PR sticks to theexisting convention and is non-breaking.
Versioning / migration
start_time_updatedstring in
stream_events.event_type).How to test locally
Summary by CodeRabbit
New Features
Bug Fixes
Tests