-
-
Notifications
You must be signed in to change notification settings - Fork 109
fix(put): use hop-by-hop routing instead of embedded origin field #2243
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
Merged
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
4ef2f37
fix(put): use hop-by-hop routing instead of embedded origin field
sanity 4b2ec5a
refactor(put): simplify to Request/Response pattern (~60% code reduct…
sanity d3b154b
fix(put): restore subscribe parameter functionality
sanity 68e2b4f
fix(put): restore subscribe parameter functionality
sanity dc12f33
fix(put): trigger Update when PUT changes state on subscribed contract
sanity 0c52318
refactor: complete hop-by-hop routing for all operations
sanity 67f7e40
fix: address race condition and block_on issues in hop-by-hop routing
sanity 5add854
fix(get): extract target_addr from current_target for forward Requests
sanity 0895dc5
refactor: simplify Update messages and rename target to next_hop
sanity f053f16
fix: enable UPDATE hop-by-hop routing via notify_op_change
sanity 53b05be
fix: UPDATE hop-by-hop routing - fix timing issue where operation was…
sanity c73e058
fix: use unique temp directories in store tests to avoid permission c…
sanity 5529c99
fix: race condition in hop-by-hop routing where state saved after send
sanity a729a03
ci: enable transport debug logging to diagnose packet delivery issues
sanity 5fcc35c
fix(ci): allow info level for test_utils in RUST_LOG
sanity f88d3e9
ci: re-trigger CI to check for flaky test
sanity bc7ba23
ci: re-run tests (attempt 2)
sanity b6b3cc4
refactor: rename target_addr to next_hop_addr for clarity
sanity 5893baf
ci: enable info logging for PUT operations to diagnose test failure
sanity e62c9bd
ci: trigger re-run to verify test stability
sanity 6254e43
ci: add diagnostic println for connectivity test mesh check
sanity df59bd9
ci: add more diagnostic logging for pub_key update and QueryConnections
sanity b837f90
fix: resolve address mismatch in handle_connect_peer for transient pr…
sanity 2e32f13
feat: improve operation lifecycle logging for CI debugging
sanity 6a3b438
ci: use JSON log format for structured output
sanity a67df53
ci: add op_state_manager and connect to RUST_LOG for timeout visibility
sanity 18029d9
ci: add target_connections and ttl to connect start log
sanity 1842f2f
test: add target_connections logging to debug flaky connect tests
sanity 4b246ed
test: force connection teardown to catch NAT routing bugs determinist…
sanity 1cf3229
Revert "test: force connection teardown to catch NAT routing bugs det…
sanity 118bfcb
fix(put): store next_hop in AwaitingResponse state for routing
sanity ed60048
ci: remove debug logging from CI workflow
sanity a9ef9a3
build: remove broken wiki submodule reference
sanity 4890a1f
refactor(test): improve structured logging in test_put_with_subscribe…
sanity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Large diffs are not rendered by default.
Oops, something went wrong.
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this removed ? we no longer initiate puts from the executor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
PutContractstruct was already marked#[allow(unused)]- it was dead code that was never called. The executor initiates PUTs through a different path (viaClientRequest::ContractOp→handle_client_request), not through theComposeNetworkMessagetrait.I removed it as part of the code cleanup since it added ~30 lines of unused code that would need to be maintained. The
SubscribeContractandUpdateContractstructs are kept because they're actually used.[AI-assisted - Claude]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the git history more closely:
PutContractwas originally added and used by you in commit c0d8230 ("Revamp upsert contract logic at wasm executor", Dec 2023). It was used to initiate PUT operations from the executor viaself.op_request(request).However, the usage was removed in commit 1e9ee5a ("Break executor source and some other changes", also Dec 2023) - that commit removed the
op_request(PutContract)calls but left the struct definition behind with#[allow(unused)].So the struct has been dead code since that refactor ~2 years ago. The executor currently initiates PUTs through a different path (via
ClientRequest::ContractOphandling in the node event loop).Should we restore the executor's ability to initiate PUTs via
ComposeNetworkMessage? If so, I can add that back. Otherwise, the dead code removal seems appropriate.[AI-assisted - Claude]