Bitmessage/PyBitmessage-2025-02-23
Archived
2
0
Fork 0
Code Issues 167 Pull Requests 54 Actions Packages Projects Releases 5 Wiki Activity

Unused code in shared and protocol modules #1374

New Issue
Closed
opened 2018-10-12 16:04:23 +02:00 by g1itch · 6 comments
g1itch commented 2018-10-12 16:04:23 +02:00 (Migrated from github.com)
Copy Link

Hello.

The functions checkAndShareObjectWithPeers, _checkAndShareUndefinedObjectWithPeers, _checkAndShareMsgWithPeers, _checkAndShareGetpubkeyWithPeers, _checkAndSharePubkeyWithPeers, _checkAndShareBroadcastWithPeers are same in protocol and shared modules and used from latter as I can see.

Remove it from protocol?

Hello. The functions `checkAndShareObjectWithPeers`, `_checkAndShareUndefinedObjectWithPeers`, `_checkAndShareMsgWithPeers`, `_checkAndShareGetpubkeyWithPeers`, `_checkAndSharePubkeyWithPeers`, `_checkAndShareBroadcastWithPeers` are same in `protocol` and `shared` modules and used from latter as I can see. Remove it from `protocol`?
g1itch commented 2018-10-12 16:47:19 +02:00 (Migrated from github.com)
Copy Link

The only place where checkAndShareObjectWithPeers() appears is class_objectProcessor.py#L751 but broadcastToSendDataQueues seems to be useless, because I cannot find where sendDataQueues are handled.

The only place where `checkAndShareObjectWithPeers()` appears is [class_objectProcessor.py#L751](../blob/v0.6/src/class_objectProcessor.py#L751) but `broadcastToSendDataQueues` seems to be useless, because I cannot find where `sendDataQueues` are handled.
PeterSurda commented 2018-10-12 17:26:52 +02:00 (Migrated from github.com)
Copy Link

In general shared should be reduced as much as possible. Just check that the code in protocol is working correctly and nothing references the removed functions from shared.

Regarding check*, this is supposed to be obsoleted because there are new check implementations in BMObject. As far as I can see, there is only one reference to them, from class_objectProcesor when you extract an ACK from a message, you call these checks before putting the ACK into the inventory. This should be refactored to use BMObject (maybe some extra coding is necessary, I'm not sure if they are compatible, I think I didn't pay enough attention during moving to asyncore), and the checks from both protocol and shared should be removed.

Regarding broadcastToSendDataQueues and sendDataQueues, I'm pretty sure they are obsolete as asyncore doesn't use them anymore. Maybe they are even creating a memory leak.

In general `shared` should be reduced as much as possible. Just check that the code in `protocol` is working correctly and nothing references the removed functions from shared. Regarding `check*`, this is supposed to be obsoleted because there are new check implementations in `BMObject`. As far as I can see, there is only one reference to them, from `class_objectProcesor` when you extract an ACK from a message, you call these checks before putting the ACK into the inventory. This should be refactored to use `BMObject` (maybe some extra coding is necessary, I'm not sure if they are compatible, I think I didn't pay enough attention during moving to asyncore), and the checks from both `protocol` and `shared` should be removed. Regarding `broadcastToSendDataQueues` and `sendDataQueues`, I'm pretty sure they are obsolete as asyncore doesn't use them anymore. Maybe they are even creating a memory leak.
PeterSurda commented 2018-10-12 17:33:25 +02:00 (Migrated from github.com)
Copy Link

I looked at it and it definitely does need extra work as the BMObject constructor requires decoded arguments, and the decoder only works on buffers. So some redesign is necessary if we're to avoid duplication of code.

I looked at it and it definitely does need extra work as the `BMObject` constructor requires decoded arguments, and the decoder only works on buffers. So some redesign is necessary if we're to avoid duplication of code.
PeterSurda commented 2018-10-12 17:37:47 +02:00 (Migrated from github.com)
Copy Link

Maybe create a fake BMProto object o, copy the buffer from the ACK buffer (or just reference it), and call o.bm_command_object() to process it. Probably some additional tests are necessary.

Maybe create a fake `BMProto` object `o`, copy the buffer from the ACK buffer (or just reference it), and call `o.bm_command_object()` to process it. Probably some additional tests are necessary.
g1itch commented 2018-10-12 17:41:38 +02:00 (Migrated from github.com)
Copy Link

I found that I do not understand where objects are broadcasted to the network ):

I found that I do not understand where objects are broadcasted to the network ):
g1itch commented 2018-10-18 09:06:13 +02:00 (Migrated from github.com)
Copy Link

I have a rather controversial solution: 03ee14e9.

I have a rather controversial solution: 03ee14e9.
This repo is archived. You cannot comment on issues.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Something isn't working as it's supposed to
build
Related to building or build system
dependencies
Pull requests that update a dependency file
developers
The issue is relevant mainly for developers rather than users
documentation
duplicate
enhancement
New feature
formatting
Code or UI text formatting
invalid
legal
mobile
Frontend for mobile devices (kivy)
obsolete
packaging
performance
protocol
Protocol change
question
refactoring
regression
Some issue, fixed in the past, appears again
security
test
translation
usability
wontfix
No Label
bug
obsolete
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
PeterSurda
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Bitmessage/PyBitmessage-2025-02-23#1374
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?