feat: persistent tx-pool data into a file when it has been shutdown

[quake]

base on #2621, using the TransactionVec struct to persistent tx-pool data directly without new schema defination.

---

This change is 

[doitian]

Does user know where the file is and how to delete it? Consider add an option in ckb reset-data to help user delete the pool persistent file and ask user to use the command like ckb reset-data --pool to delete the corrupted file.

I modified the error message, it will print the file path:

TxPool Error: The tx-pool persisted data file ["/home/xxx/ckb/data/tx_pool_persisted_data.v1"] is broken, please delete it and restart, cause: TransactionVecReader total size doesn't match, expect 1717859076, actual 8

[keroro520]

When systemctl stop ckb-node-8114, it sends SIGTERM to stop the service, will SIGTERM trigger dumping the tx-pool?

[quake]

When systemctl stop ckb-node-8114, it sends SIGTERM to stop the service, will SIGTERM trigger dumping the tx-pool?

Yes

[quake]

What does generate_transaction() does? Why this transaction is special that we cannot create it in the loop?

the doc of generate_transaction():

// generate a transaction which spend tip block's cellbase and send it to pool through rpc.

[quake]

@doitian @keroro520 @zhangsoledad I have rebased this PR with develop branch and resolved issues accordingly, please review again, thanks.

[doitian]

Minor and Nit (Not important thing) mean the suggestions do not block the PR.

Reviewed 7 of 28 files at r3, 22 of 22 files at r4.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @keroro520, @quake, and @zhangsoledad)

---

test/src/node.rs, line 30 at r4 (raw file):

struct ProcessGuard {
    pub child: Child,

Minor: Use Option<Child>

---

tx-pool/src/pool.rs, line 184 at r4 (raw file):

    }

    /// Add tx which proposed but still uncommittable to gap pool

Nit, add comment about the return value.

---

tx-pool/src/service.rs, line 407 at r4 (raw file):

    }

    /// send suspend chunk process cmd

Minor

-   /// send suspend chunk process cmd
+   /// Sends suspend chunk process cmd 

---

tx-pool/src/service.rs, line 423 at r4 (raw file):
Minor

    /// Saves tx pool into disk.

---

tx-pool/src/service.rs, line 440 at r4 (raw file):

    fn load_persisted_data(&self, mut txs: Vec<TransactionView>) -> Result<(), AnyError> {
        if !txs.is_empty() {

Minor, early return reduces indentation.

---

tx-pool/src/service.rs, line 442 at r4 (raw file):

        if !txs.is_empty() {
            info!("Loading persisted tx-pool data, total {} txs", txs.len());
            // a trick to commit transactions with the correct order

Minor, explain what's the trick:

In each loop, try to add all the remaining transactions. Remove the successfully added transactions.

Stop the loop until there's no transactions to add in an iteration.

---

tx-pool/src/service.rs, line 444 at r4 (raw file):

            // a trick to commit transactions with the correct order
            loop {
                let mut failed_txs = Vec::new();

We can use Vec::drain to leave only the failed txs. Just remember the vec size before drain. If the size does not change, early return the loop. The loop also can use while !txs.empty() instead.

---

tx-pool/src/service.rs, line 461 at r4 (raw file):

                    break;
                }
                txs = failed_txs;

FYI, when I first review the code, it confused me because the assignment is in the last of loop. I don't know it when I reviewed the two break checks. Renaming the variable txs to remaining_txs can help, but using while !txs.empty() is better to give the hint that we are slowly draining the vector.

[doitian]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @keroro520, @quake, and @zhangsoledad)

---

tx-pool/src/service.rs, line 423 at r4 (raw file):

Previously, doitian (ian) wrote…

Minor

    /// Saves tx pool into disk.

Keep consistent: saves -> Saves

---

tx-pool/src/service.rs, line 444 at r4 (raw file):

Previously, doitian (ian) wrote…

We can use Vec::drain to leave only the failed txs. Just remember the vec size before drain. If the size does not change, early return the loop. The loop also can use while !txs.empty() instead.

Sorry, it should beVec::retain

[quake]

@doitian @zhangsoledad updated accordingly to your comments, please review again, thanks.

[doitian]

Will the dump be saved as JSON or Molecule binary?

[quake]

Will the dump be saved as JSON or Molecule binary?

molecule binary

[doitian]

Reviewed 13 of 13 files at r5.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @keroro520, @quake, and @zhangsoledad)

---

tx-pool/src/service.rs, line 443 at r5 (raw file):

        if !txs.is_empty() {
            info!("Loading persisted tx-pool data, total {} txs", txs.len());
            let mut failed_txs = 0;

Minor suggestion: Iter::all can simplify the code here.

[doitian]

@zhangsoledad @keroro520 please review

[doitian]

Hold to merge it locally.

Still waiting for another reviewer.

[doitian]

Merged in cc665b3