Uh oh!
There was an error while loading. Please reload this page.
Transaction gossiping - #29
Conversation
e97f4bf to
7777439Compare5cf26e4 to
635cdb7Compare1cb15f9 to
1fb6bd4Compare1fb6bd4 to
cc64246CompareCodecov Report
@@ Coverage Diff @@## main #29 +/- ##
===========================================
- Coverage 71.81% 41.02% -30.80%
===========================================
Files 3 4 +1 Lines 110 234 +124 ===========================================
+ Hits 79 96 +17 - Misses 16 121 +105 - Partials 15 17 +2
Continue to review full report at Codecov.
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| * error prone | ||
| * Re-using other mempool (Celo, Prysm, etc) | ||
| * different API | ||
| * potential licensing issues |
There was a problem hiding this comment.
We should still list cons even, or especially, for the decided for approach (of using the tm mempool). Cons I can think of:
- inherit all limitations of the tendermint mempool (no prioritization of Txs - maybe list some issues in TM/TM-specs)
- legacy code base (the tendermint mempool exists for a while now)
- ...
db9d945 to
aff4203CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| time.Sleep(5 * time.Second) | ||
| // gossip from client 4 | ||
| err := clients[4].GossipTx(ctx, []byte("sample tx")) |
There was a problem hiding this comment.
So the problem seems to be that the message we are trying to publish does not get validated properly: https://github.com/libp2p/go-libp2p-pubsub/blob/5457a2845b06d859df6d3683867808e46a15c500/validation.go#L242-L247
| assert.NoError(err) | ||
| nCtx, nCancel := context.WithTimeout(ctx, 1*time.Second) | ||
| nCtx, nCancel := context.WithTimeout(ctx, 300*time.Second) |
There was a problem hiding this comment.
Wowser. Does that mean, the messages gets now written to the channel but the client isn't able to retrieve it now?
There was a problem hiding this comment.
This is a work in progress, big value to give me more time in debugger ;)
There was a problem hiding this comment.
Doesn't a breakpoint stop the "world"? Or does that depend on which debugger you are using?
| const reAdvertisePeriod = 1 * time.Hour | ||
| const ( | ||
| // reAdvertisePeriod defines a period after which P2P client re-attempt advertising namespace in DHT. | ||
| reAdvertisePeriod = 1 * time.Hour |
There was a problem hiding this comment.
That is so long because once they've advertized this it won't change?
liamsi
commented
Mar 25, 2021
It seems the nodes are not really connected with each other now. |
I expect this to work, but it doesn't.
4f129ce to
a0ef894Comparea0ef894 to
4683436CompareNumber 003 is selected, because 002 is proposed in #29.
| } | ||
| func NewNode(ctx context.Context, conf config.NodeConfig, nodeKey crypto.PrivKey, clientCreator proxy.ClientCreator, logger log.Logger) (*Node, error) { | ||
| func NewNode(ctx context.Context, conf config.NodeConfig, nodeKey crypto.PrivKey, clientCreator proxy.ClientCreator, genesis *types.GenesisDoc, logger log.Logger) (*Node, error) { |
There was a problem hiding this comment.
optional nit: IMO a line break per parameter would increase readability.
| type Tx struct { | ||
| Data []byte | ||
| } |
There was a problem hiding this comment.
IMO, we will most likely keep it that way or even make it an alias like in tendermint: https://github.com/tendermint/tendermint/blob/9ecfcc93a6364bbecc9e1d7740b53602eda667eb/types/tx.go#L14-L17
Reason is the abstraction between tendermint (and with it optimint) and the abci app (that understands the Tx).
It is certainly not the right place for the type definition though but that will be part of that refactoring mentioned in the todo I guess.
| // wait for clients that should receive Tx | ||
| wg.Wait() | ||
| } |
| select { | ||
| case <-ctx.Done(): | ||
| return ctx.Err() | ||
| case <-c.dht.RefreshRoutingTable(): | ||
| } |
| // TODO(tzdybal): is there a better way to wait for readiness? | ||
| time.Sleep(1 * time.Second) |
There was a problem hiding this comment.
Nope) Even tests in libp2p use sleep but with much less time) Though I've seen discussions about fixing this several time thought more than a year)
There was a problem hiding this comment.
1s is empirical result for GitHub actions ;) Locally even 100ms works.
Transaction gossiping, using
Resolves#3.
Depends on #17 and #30.