GH-631: Create FailedPayables Table
#646
Conversation
| let txs = vec![tx1, tx2, tx3]; | ||
| let txs = vec![tx1, tx2]; | ||
|
|
||
| let result = subject.insert_new_records(&txs); |
There was a problem hiding this comment.
We were discussing that maybe the code should actually panic if an already confirmed transaction (having the block field populated) is supplied.
There was a problem hiding this comment.
I think it would be too restrictive, maybe we want to insert confirmed transactions too.
There was a problem hiding this comment.
Maybe yes, but please don't write unrealistic tests.
We know exactly, that our algorithm will always put in a transaction that is not confirmed at the time we are creating the record.
If I were writing this, I wouldn't hesitate and include the panic constraint. What we are writing is a complex system where every little mistake can break something. I wish I could know of each behavior that we didn't consider as possible. That's why use unreachable!() so much these days. Those are border lines for me, which I don't think we step over but if we do then the code need a serious fix.
That's my take.
| "0x2345678901234567890123456789012345678901234567890123456789012345", | ||
| rusqlite::types::Null, | ||
| ]) | ||
| .unwrap(); |
There was a problem hiding this comment.
Wouldn't it be simpler if you used the normal insert_new_records() fn and then used the update sql statement just to null out the single cell? I'd expect much less code.
There was a problem hiding this comment.
I'm doing it because of the wrapped_conn, it's complex to reuse it later.
There was a problem hiding this comment.
There is no conflict though.
If you can call .prepare() and then .execute() on it, then you can do whatever you want.
It would allow you to run the update statement.
Really, try it.
subject
.insert_new_records(&vec![tx.clone()])
wrapped_conn
.prepare("UPDATE failed_payable (block_number) values (null)")
.unwrap()
.execute([])
.unwrap();
It should work... but my brain has its circuits in flames at the moment 🤷♂️
| let faulty_insert_stmt = { setup_conn.prepare("SELECT id FROM example").unwrap() }; | ||
| let wrapped_conn = ConnectionWrapperMock::default() | ||
| .prepare_result(Ok(get_tx_identifiers_stmt)) | ||
| .prepare_result(Ok(faulty_insert_stmt)); |
There was a problem hiding this comment.
Here you are preparing for two transactions but eventually the test performs over just one.
There was a problem hiding this comment.
It'll require both of them. Since, we need to check for unique hashes.
No description provided.