refactor: store TransactionContext in TransactionRecord#582
Open
xdustinface wants to merge 3 commits intov0.42-devfrom
Open
refactor: store TransactionContext in TransactionRecord#582xdustinface wants to merge 3 commits intov0.42-devfrom
TransactionContext in TransactionRecord#582xdustinface wants to merge 3 commits intov0.42-devfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughConsolidates block confirmation metadata into a new FFI-safe Changes
Sequence Diagram(s)sequenceDiagram
participant C as C caller (FFI)
participant F as FFI layer
participant T as Context Helper
participant W as Wallet/Core
C->>F: wallet_check_transaction(tx_bytes, context_type, block_info, ...)
F->>T: transaction_context_from_ffi(context_type, &block_info)
alt context conversion fails
T-->>F: None
F-->>C: set FFIError InvalidInput, return false
else context ok
T-->>F: Some(TransactionContext)
F->>W: perform transaction check with TransactionContext
W-->>F: FFITransactionCheckResult
F-->>C: return true + result_out
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #582 +/- ##
=============================================
+ Coverage 67.44% 67.54% +0.10%
=============================================
Files 318 318
Lines 67020 67059 +39
=============================================
+ Hits 45204 45298 +94
+ Misses 21816 21761 -55
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
key-wallet/src/managed_account/mod.rs (1)
377-396:⚠️ Potential issue | 🟠 MajorReturn
truewhen reprocessing mutates UTXOs.This path now sets
changedonly fromtx_record.context != context. If a gap-limit rescan reprocesses the same context butupdate_utxos()adds/removes wallet outputs, the account state changes while this still returnsfalse, which can suppress downstream balance/persistence work. Haveupdate_utxos()report whether it mutated state and OR that intochanged. As per coding guidelines "Perform atomic state updates: when adding transactions, always update all related state atomically including address usage state, UTXO set, transaction history, and balance calculations".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet/src/managed_account/mod.rs` around lines 377 - 396, The confirm_transaction path currently only sets changed when tx_record.context != context, missing mutations from update_utxos; modify update_utxos to return a bool indicating whether it mutated UTXO/account state and in confirm_transaction capture that return value and OR it into changed (i.e., let utxos_changed = self.update_utxos(tx, account_match, context); changed |= utxos_changed), keeping the existing record_transaction and tx_record.update_context logic intact so all related state updates remain atomic.key-wallet/src/managed_account/transaction_record.rs (1)
15-23:⚠️ Potential issue | 🔴 CriticalKeep
TransactionRecordbackward-deserializable.With
serdeenabled, this replaces the persisted{height, block_hash, timestamp}layout with a requiredcontextfield. Older wallet/account bytes won't containcontext, so deserialization will fail beforeManagedCoreAccountcan rebuildspent_outpoints. Please add a compatibility layer here (customDeserialize, versioned format, or an explicit migration) before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet/src/managed_account/transaction_record.rs` around lines 15 - 23, TransactionRecord's new required field `context` breaks deserialization of older persisted records; implement a compatibility deserializer for TransactionRecord that accepts the old shape (height, block_hash, timestamp) as optional fields and constructs a TransactionContext when `context` is missing. Concretely, add a custom Deserialize impl (or serde with #[serde(deserialize_with = "...")] for TransactionRecord) that tries to read `context` first, and if absent reads `height`, `block_hash`, `timestamp` and synthesizes a TransactionContext (using the same enum/struct constructors as TransactionContext) before returning the TransactionRecord; ensure the code references the TransactionRecord type, the TransactionContext constructor(s), and the field names `transaction`, `txid`, and `context` so older bytes remain readable and ManagedCoreAccount rebuilding logic continues to work.key-wallet-ffi/src/transaction.rs (1)
388-397:⚠️ Potential issue | 🟠 MajorFFI signature change:
wallet_check_transactionparameter consolidation.Same pattern as
managed_wallet_check_transaction- the function now acceptsblock_info: FFIBlockInfoby value instead of separateblock_height,block_hash, andtimestampparameters.This maintains consistency across the FFI API surface. Ensure downstream consumers are updated for both functions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet-ffi/src/transaction.rs` around lines 388 - 397, The FFI signature for wallet_check_transaction was changed to take a consolidated FFIBlockInfo struct instead of separate block_height, block_hash, and timestamp parameters; update the implementation of wallet_check_transaction to accept and use the new block_info: FFIBlockInfo parameter (match how managed_wallet_check_transaction handles block_info), replace references to the old individual parameters with block_info.<field> accesses, and ensure the FFITransactionCheckResult population and any validation use block_info consistently so the API matches managed_wallet_check_transaction.key-wallet-ffi/src/transaction_checking.rs (1)
109-119:⚠️ Potential issue | 🟠 MajorFFI signature change:
managed_wallet_check_transactionparameter consolidation.The function signature changed from separate parameters (
block_height,block_hashpointer,timestamp) to a singleblock_info: FFIBlockInfopassed by value. This is an ABI-breaking change.The implementation is correct:
FFIBlockInfois#[repr(C)]andCopy, making it safe to pass by value- The 40-byte struct (4 + 32 + 4 bytes) is reasonable for stack passing
Ensure C/Swift/Kotlin callers are updated to construct and pass
FFIBlockInfoinstead of separate arguments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet-ffi/src/transaction_checking.rs` around lines 109 - 119, The FFI changed: managed_wallet_check_transaction now takes a single FFIBlockInfo (by value) instead of separate block_height, block_hash pointer, and timestamp, which is an ABI-breaking change; update all native callers (C/Swift/Kotlin) to construct and pass an FFIBlockInfo value to managed_wallet_check_transaction, ensuring the struct layout matches the Rust definition (FFIBlockInfo is #[repr(C)] and Copy with 4+32+4 bytes) and adjust any call sites that previously passed the three separate parameters to instead populate and pass the FFIBlockInfo instance.key-wallet-ffi/src/managed_account.rs (1)
662-675:⚠️ Potential issue | 🟠 MajorABI-breaking change:
FFITransactionRecordstruct layout has changed.The struct previously had flat fields (inferred as
height,block_hash,timestampbased on the commit message "Replace separate block-related fields") that are now nested insidecontext: FFITransactionContextDetails. This changes the C struct memory layout and requires updates to FFI consumers.
- Before: Consumers accessed flat transaction block info directly
- After: Consumers must access
record->context.block_info.height, etc.Any C/Swift/Kotlin bindings reading the old field offsets will receive garbage data. Ensure:
- FFI header files are regenerated using cbindgen
- All downstream consumers (Swift, Kotlin, C) are updated to use the new nested field paths
- This breaking change is documented in release notes with migration guidance
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet-ffi/src/managed_account.rs` around lines 662 - 675, FFITransactionRecord's memory layout changed by moving flat block fields into the nested context: FFITransactionContextDetails, so update consumers to read block info via record->context.block_info.height / .block_hash / .timestamp (or equivalent language bindings), regenerate FFI headers with cbindgen to reflect the new layout, update Swift/Kotlin/C binding access paths to the nested fields, and add a release-note entry documenting the ABI-breaking change and migration steps for downstream users.key-wallet-ffi/FFI_API.md (1)
855-862:⚠️ Potential issue | 🟠 MajorRefresh the generated prose for the new
FFIBlockInfoABI.The signatures now take
block_info, but the surrounding text is still pre-refactor. Line 859 omitscontext_type,block_info, andupdate_state, while Line 1307 still documents removed out-params instead ofresult_out. The document also never explains theFFIBlockInfofields, so callers cannot tell how to populate the new parameter. Since this file is auto-generated, the fix needs to happen in the Rust doc comments or generator inputs.Also applies to: 1303-1310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet-ffi/FFI_API.md` around lines 855 - 862, Update the generated documentation in the Rust doc comments / generator inputs for the managed_wallet_check_transaction FFI: include the new parameters (context_type: FFITransactionContext, block_info: FFIBlockInfo, update_state: bool) in the prose, replace any removed out-param descriptions with the single result_out pointer, and explicitly document that result_out must be populated and freed via transaction_check_result_free; additionally add a clear description of the FFIBlockInfo struct fields (what each field means and how callers should populate them) so callers can build the new parameter. Also make the same updates to the other transaction-check doc block that still references removed out-params so both generated sections reflect the new ABI.
🧹 Nitpick comments (1)
key-wallet-ffi/src/wallet_manager_tests.rs (1)
620-689: These tests still short-circuit before the new context mapping runs.All three calls use malformed
tx_bytes, sowallet_manager_process_transaction()can fail during transaction parsing without ever touchingFFITransactionContextDetails/FFIBlockInfo. That leaves the newin_block,in_chain_locked_block, andinstant_sendpaths effectively untested. Please add at least one valid serialized transaction case per context or a direct unit test around the conversion helpers. As per coding guidelines "Write unit tests for new functionality in Rust code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet-ffi/src/wallet_manager_tests.rs` around lines 620 - 689, The tests currently use malformed tx_bytes so wallet_manager::wallet_manager_process_transaction returns before exercising the new FFITransactionContextDetails mapping; add at least one valid serialized transaction for each context (mempool, FFITransactionContextDetails::in_block, FFITransactionContextDetails::in_chain_locked_block / instant_send) so the code reaches the context conversion path, or alternatively add direct unit tests around the conversion helpers that translate FFITransactionContextDetails/FFIBlockInfo into the internal context types; specifically update the tests that call wallet_manager_process_transaction (and/or add new tests for the conversion functions) to provide well-formed tx_bytes and assert that the correct internal context mapping/flags are produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@key-wallet-ffi/src/types.rs`:
- Around line 21-29: FFIBlockInfo::empty() is being treated as a valid confirmed
block; update transaction_context_from_ffi() (and any FFI entry points that call
it) so that when the incoming FFIBlockInfo equals FFIBlockInfo::empty() you
reject it for confirmed contexts (InBlock and InChainLockedBlock) by returning
an Err/None and have to_transaction_context() and the public FFI wrappers map
that to FFIErrorCode::InvalidInput instead of creating a zero-hash confirmed
context; ensure only unconfirmed contexts accept the empty sentinel and
reference the functions transaction_context_from_ffi, to_transaction_context,
and the FFI entry points that call them when making the change.
---
Outside diff comments:
In `@key-wallet-ffi/FFI_API.md`:
- Around line 855-862: Update the generated documentation in the Rust doc
comments / generator inputs for the managed_wallet_check_transaction FFI:
include the new parameters (context_type: FFITransactionContext, block_info:
FFIBlockInfo, update_state: bool) in the prose, replace any removed out-param
descriptions with the single result_out pointer, and explicitly document that
result_out must be populated and freed via transaction_check_result_free;
additionally add a clear description of the FFIBlockInfo struct fields (what
each field means and how callers should populate them) so callers can build the
new parameter. Also make the same updates to the other transaction-check doc
block that still references removed out-params so both generated sections
reflect the new ABI.
In `@key-wallet-ffi/src/managed_account.rs`:
- Around line 662-675: FFITransactionRecord's memory layout changed by moving
flat block fields into the nested context: FFITransactionContextDetails, so
update consumers to read block info via record->context.block_info.height /
.block_hash / .timestamp (or equivalent language bindings), regenerate FFI
headers with cbindgen to reflect the new layout, update Swift/Kotlin/C binding
access paths to the nested fields, and add a release-note entry documenting the
ABI-breaking change and migration steps for downstream users.
In `@key-wallet-ffi/src/transaction_checking.rs`:
- Around line 109-119: The FFI changed: managed_wallet_check_transaction now
takes a single FFIBlockInfo (by value) instead of separate block_height,
block_hash pointer, and timestamp, which is an ABI-breaking change; update all
native callers (C/Swift/Kotlin) to construct and pass an FFIBlockInfo value to
managed_wallet_check_transaction, ensuring the struct layout matches the Rust
definition (FFIBlockInfo is #[repr(C)] and Copy with 4+32+4 bytes) and adjust
any call sites that previously passed the three separate parameters to instead
populate and pass the FFIBlockInfo instance.
In `@key-wallet-ffi/src/transaction.rs`:
- Around line 388-397: The FFI signature for wallet_check_transaction was
changed to take a consolidated FFIBlockInfo struct instead of separate
block_height, block_hash, and timestamp parameters; update the implementation of
wallet_check_transaction to accept and use the new block_info: FFIBlockInfo
parameter (match how managed_wallet_check_transaction handles block_info),
replace references to the old individual parameters with block_info.<field>
accesses, and ensure the FFITransactionCheckResult population and any validation
use block_info consistently so the API matches managed_wallet_check_transaction.
In `@key-wallet/src/managed_account/mod.rs`:
- Around line 377-396: The confirm_transaction path currently only sets changed
when tx_record.context != context, missing mutations from update_utxos; modify
update_utxos to return a bool indicating whether it mutated UTXO/account state
and in confirm_transaction capture that return value and OR it into changed
(i.e., let utxos_changed = self.update_utxos(tx, account_match, context);
changed |= utxos_changed), keeping the existing record_transaction and
tx_record.update_context logic intact so all related state updates remain
atomic.
In `@key-wallet/src/managed_account/transaction_record.rs`:
- Around line 15-23: TransactionRecord's new required field `context` breaks
deserialization of older persisted records; implement a compatibility
deserializer for TransactionRecord that accepts the old shape (height,
block_hash, timestamp) as optional fields and constructs a TransactionContext
when `context` is missing. Concretely, add a custom Deserialize impl (or serde
with #[serde(deserialize_with = "...")] for TransactionRecord) that tries to
read `context` first, and if absent reads `height`, `block_hash`, `timestamp`
and synthesizes a TransactionContext (using the same enum/struct constructors as
TransactionContext) before returning the TransactionRecord; ensure the code
references the TransactionRecord type, the TransactionContext constructor(s),
and the field names `transaction`, `txid`, and `context` so older bytes remain
readable and ManagedCoreAccount rebuilding logic continues to work.
---
Nitpick comments:
In `@key-wallet-ffi/src/wallet_manager_tests.rs`:
- Around line 620-689: The tests currently use malformed tx_bytes so
wallet_manager::wallet_manager_process_transaction returns before exercising the
new FFITransactionContextDetails mapping; add at least one valid serialized
transaction for each context (mempool, FFITransactionContextDetails::in_block,
FFITransactionContextDetails::in_chain_locked_block / instant_send) so the code
reaches the context conversion path, or alternatively add direct unit tests
around the conversion helpers that translate
FFITransactionContextDetails/FFIBlockInfo into the internal context types;
specifically update the tests that call wallet_manager_process_transaction
(and/or add new tests for the conversion functions) to provide well-formed
tx_bytes and assert that the correct internal context mapping/flags are
produced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c04cb4e1-b1f0-4054-bea3-e4b2535e833e
📒 Files selected for processing (14)
key-wallet-ffi/FFI_API.mdkey-wallet-ffi/src/managed_account.rskey-wallet-ffi/src/transaction.rskey-wallet-ffi/src/transaction_checking.rskey-wallet-ffi/src/types.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/transaction_record.rskey-wallet/src/tests/spent_outpoints_tests.rskey-wallet/src/transaction_checking/transaction_context.rskey-wallet/src/transaction_checking/transaction_router/tests/helpers.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/transaction_checking/wallet_checker.rs
xdustinface
added a commit
that referenced
this pull request
Mar 25, 2026
Addresses CodeRabbit review comment on PR #582 #582 (comment)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@key-wallet-ffi/src/types.rs`:
- Around line 48-71: Add a #[cfg(test)] unit-test module next to
transaction_context_from_ffi that exercises all FFITransactionContext variants
using both a FFIBlockInfo::empty() and a non-empty FFIBlockInfo: assert that
transaction_context_from_ffi(FFITransactionContext::InBlock,
&FFIBlockInfo::empty()) and the InChainLockedBlock variant return None, and
assert that Mempool and InstantSend return
Some(TransactionContext::Mempool/InstantSend) even when given
FFIBlockInfo::empty(), plus validate that InBlock/InChainLockedBlock return
Some(...) for a non-empty block_info; structure as a small table-driven test to
cover accepted and rejected paths.
- Around line 31-35: FFIBlockInfo::to_block_info is currently infallible and
only rejects the exact zero-hash sentinel; change it to be fallible (return
Result<BlockInfo, Error> or Option<BlockInfo>) and validate the full invariant:
require the block_hash not equal to the all-zero byte array and require
timestamp != 0 before calling BlockInfo::new. Update all call sites (where
FFIBlockInfo is used to build confirmed TransactionContext or any "block-based"
context) to handle the failure path and avoid constructing confirmed contexts
when either hash or timestamp are zero; apply the same change for the related
conversions in the 52-71 region.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 823f9078-b257-44b4-86c3-774e8b0a02df
📒 Files selected for processing (4)
key-wallet-ffi/src/transaction.rskey-wallet-ffi/src/transaction_checking.rskey-wallet-ffi/src/types.rskey-wallet-ffi/src/wallet_manager.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- key-wallet-ffi/src/transaction_checking.rs
- key-wallet-ffi/src/transaction.rs
|
!fulltest |
dashinfraclaw
added a commit
to dashinfraclaw/platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
4 tasks
dashinfraclaw
added a commit
to dashinfraclaw/kotlin-platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/ferment
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/quorum-list-server
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/kotlin-platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/kotlin-platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
|
🔧 Infraclaw fulltest results for Testing rust-dashcore PR #582 (
Platform failure analysis: Updated manually after agent session interrupted. |
dashinfraclaw
added a commit
to dashinfraclaw/platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/kotlin-platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/quorum-list-server
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/ferment
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
dashinfraclaw
added a commit
to dashinfraclaw/kotlin-platform
that referenced
this pull request
Mar 26, 2026
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
xdustinface
added a commit
that referenced
this pull request
Mar 26, 2026
Addresses CodeRabbit review comment on PR #582 #582 (comment)
xdustinface
added a commit
that referenced
this pull request
Mar 26, 2026
Addresses CodeRabbit review comment on PR #582 #582 (comment)
xdustinface
added a commit
that referenced
this pull request
Mar 26, 2026
Addresses CodeRabbit review comment on PR #582 #582 (comment)
xdustinface
force-pushed
the
refactor/store-transaction-context-in-transaction-record
branch
from
c4c0240 to
7c0acad
Compare
xdustinface
added a commit
that referenced
this pull request
Mar 26, 2026
Addresses CodeRabbit review comment on PR #582 #582 (comment)
xdustinface
added a commit
that referenced
this pull request
Mar 26, 2026
Addresses CodeRabbit review comment on PR #582 #582 (comment)
xdustinface
force-pushed
the
refactor/store-transaction-context-in-transaction-record
branch
from
7c0acad to
bd37091
Compare
Replace separate block-related fields with a `context: TransactionContext` in `TransactionRecord`, preserving the full transaction context (mempool, instant-send, in-block, chain-locked). Replace `mark_confirmed`/`mark_unconfirmed` with a single `update_context` method. Introduce `FFIBlockInfo` struct and use `FFITransactionContextDetails` in `FFITransactionRecord` to expose the full context across the FFI boundary.
Addresses CodeRabbit review comment on PR #582 #582 (comment)
Addresses CodeRabbit review comment on PR #582 #582 (comment)
xdustinface
force-pushed
the
refactor/store-transaction-context-in-transaction-record
branch
from
bd37091 to
d8c394e
Compare
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
github-actions
bot
added
the
merge-conflict
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
3 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.
Replace separate block-related fields with a
context: TransactionContextinTransactionRecord, preserving the full transaction context (mempool, instant-send, in-block, chain-locked). Replacemark_confirmed/mark_unconfirmedwith a singleupdate_contextmethod. IntroduceFFIBlockInfostruct and useFFITransactionContextDetailsinFFITransactionRecordto expose the full context across the FFI boundary.Summary by CodeRabbit