Conversation
|
|
||
| # tmp folder | ||
|
|
||
| /tmp No newline at end of file |
There was a problem hiding this comment.
What's the use of adding /tmp?
There was a problem hiding this comment.
the logs are written to /tmp, and i would like to save the logs for debugging but not commit them to github
| TimeUpdateFrequency: 100 * time.Millisecond, | ||
| TimeUpdateAmount: 500 * time.Millisecond, | ||
| CrashInterval: 800 * time.Millisecond, | ||
| LogDirectory: "tmp", |
There was a problem hiding this comment.
I think we need to create a temporary folder and if the test succeeds, delete the logs.
There was a problem hiding this comment.
This way you don't need to delete the directory at the beginning of the test
There was a problem hiding this comment.
added deleting the tmp directory if the test fails, but I still think we should check if the directory exists at the beginning of the test. This way if the test fails, and we want to re-run we don't append to an already existing log file
| } | ||
|
|
||
| func (n *Network) StartInstances() { | ||
| panic("Call Run() Instead") |
There was a problem hiding this comment.
This method isn't used. Why is it needed?
There was a problem hiding this comment.
because BasicNetwork is embedded in Network i don't want the caller accidentally calling StartInstances thinking its a valid method
testutil/random_network/mempool.go
Outdated
| } | ||
|
|
||
| func (m *Mempool) isTxInChain(txID txID, parentDigest simplex.Digest) bool { | ||
| block, exists := m.verifiedButNotAcceptedTXs[parentDigest] |
There was a problem hiding this comment.
I'm not sure this works for a case where we have a leader failover that creates a sibling block.
Suppose block a is notarized and then a block b is built on top of it and broadcast, and verified by a node but then the majority notarize the empty block.
In the next round, block b' is proposed on top of block a.
We need a way to rollback the verified but not accepted map for the parent a because both blocks b and b' are built on top of it.
One way of doing it, is move this book-keeping to the blocks themselves.
Specifically, have the instances of the blocks maintain pointers to blocks they are built upon, and then just have the transaction list inside a block a map, where when we encode the bytes of the block, we do it across some order that is set at the time we build the block.
There was a problem hiding this comment.
We need a way to rollback the verified but not accepted map for the parent a because both blocks b and b' are built on top of it.
why do we need this? we aren't checking if the block was verified before accepting it anyways. We can do so if we want.
secondly, this should still succeed verification in your example. If we want to make the mempool more rigorous we could check if any two verified blocks have the same parent and delete the earlier block before verified the later. In your example when block b initially gets verified we do nothing. Then if block b' gets verified, we delete block b from the verification map before marking b' as verified. Then we can adjust AcceptBlock to ensure the block has been verified before hand.
There was a problem hiding this comment.
hmm, actually I think you are right. been trying to solve a bug relating to this. I dont think we can have the blocks point to other blocks however since we need to be able to serialize/deserialize blocks and this would cause some recursive headaches. Maybe its best to have the mempool keep a lookup table of built blocks and just fetch from this table using the parent digest when needed
There was a problem hiding this comment.
why do we need this? we aren't checking if the block was verified before accepting it anyways. We can do so if we want.
If you have verified a block and you haven't accepted it, but accepted a sibling, you need to put back all transactions you have removed from the mempool, back into the mempool, of the block that you ended up accepting its sibling (unless that transaction ended up in the sibling block)
|
Can we rebase on top of main? |
76e550a to
1e1fdc1
Compare
No description provided.