-
Notifications
You must be signed in to change notification settings - Fork 8
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
unconfirmed tx chaining. spend unconfirmed utxos. #197
unconfirmed tx chaining. spend unconfirmed utxos. #197
Conversation
wallet subscribes to mempool via tokio broadcast to track owned utxos. Changes: * add spent_utxos, unspent_utxos lists to WalletState struct * add WalletState::handle_mempool_event() * add test confirmed_and_unconfirmed_balance() * add Mempool::event_channel (tokio broadcast channel) * Mempool faillible mutation methods return Result() * Mempool mutable methods broadcast MempoolEvent * add tests::shared::mine_block_to_wallet() * lib.rs: spawn wallet task for listening to mempool events and dispatch to WalletState for handling. * add locks::tokio::AtomicRw::try_lock_guard_mut()
removes the mempool broadcast channel and wallet listener task. Instead all mempool mutations go through GlobalState methods which inform wallet of the changes. This makes changes atomic over mempool+wallet so they are always in sync. Changes: * remove Mempool::event_channel * Mempool &mut methods only callable by super * Mempool &mut methods return MempoolEvent(s) * add MempoolEvent::UpdateTxMutatorSet. (unused) * add GlobalState methods: mempool_clear, mempool_insert, mempool_prune_stale_transactions * remove spawn_wallet_task from lib.rs * add/improve doc-comments
closing this as it doesn't seem likely to be merged anytime soon, and also I'm told it is infeasible to chain tx, although I don't clearly understand why. Perhaps @aszepieniec can clarify? |
Short answer: where would the proof come from? Long answer: Every transaction must be supported by a
Chaining transactions means that the second operand has inputs that are outputs in the first. You cannot produce a To address this deficiency, we would need a different type of transaction and supporting proof. For instance,
With that data structure we could write the following proof pipelines:
|
thanks @aszepieniec. That's very clear. I will just note for anyone looking at this in the future that this PR was created before the fully fleshed out proof system was merged. Thus tx would validate at that time which will not validate today. |
Addresses #189
Extends #184. ( diff )
This PR implements chaining of unconfirmed transactions.
Or at least it appears to. I've been told this is not feasible without triton-vm programs.
So maybe it only appears to work, but is not fully correct. That's for reviewers to decide, and tell me how the approach is wrong. Or maybe it is only cut-through that requires tricky triton-vm stuff, and this PR doesn't attempt cut-through.
Primarily for this reason, I am making this a draft PR. Another reason is that this PR does not (yet?) attempt to handle (or test) the case where unconfirmed tx B depends on A, and B has a higher fee, so it gets confirmed in a block and A does not. I'm not certain what the code will do... undefined behavior.
The approach taken here is a natural extension of the changes in #184. Namely the wallet-state keeps track of all wallet-affecting utxos in the mempool. It considers them for balance calculations, for selecting inputs, when scanning for spent-inputs, and when updating wallet state with a new block.
The PR includes a test case, spend_unconfirmed_tx() in wallet_state.rs. The test case starts with a single input utxo then sends Tx A worth 5, then tx B worth 10 in the same block (B depends on A) and verifies balances, then mines another block and verifies balances. In detail:
steps 5 and 6 were the trickiest as it required some changes to WalletState::update_wallet_state_with_new_block() which is a complex method.
There may be better way(s) to represent the unconfirmed utxos in WalletStatus for more efficient balance calcs.
But for now, I don't want to put further effort into improving unless/until the concept/direction is approved by reviewers.
My thought is that this approach may not be the best/ideal way to achieve this functionality, but if not fatally incorrect then it may be worth having as a stop-gap for now, until a better approach can be implemented.