It seems like these request counters should be incremented?

[jla415]

Correct me if I'm wrong here... Thanks

[pocesar]

nope, not really. the currentId should only change when the commands are changed. it's used to keep track of the current request

the increment happens here https://github.com/pocesar/node-stratum/blob/master/src/client.ts#L320

[jla415]

Maybe i'm missing something but it reuses id 1 for both subscribe and authorize right now in your client example?

[jla415]

client.connect(8080, 'localhost').then(function(socket){
    socket.stratumSubscribe('NodeMiner');
    socket.stratumAuthorize('user','pass');
});

^^^ this code reuses id 1 for the authorize request and the pending subscription request is lost

[pocesar]

every time there's a call to stratumSend, the id gets incremented. both stratumSubscribe and stratumAuthorize calls stratumSend, that's why. but since you are calling them in sucession, they are executing in the same id since they are in the same even loop, so it should be:

client.connect(8080, 'localhost').then(function(socket){
    socket.stratumSubscribe('NodeMiner').then(function() {
      return socket.stratumAuthorize('user','pass');
    }).then(function() {
      // do something after authorization
    }, function() { 
      // authorization failed
    });
});

promises are always executed in the next process tick, so you need to "queue" them in a serial manner

[jla415]

in stratumSend you are only incrementing the counter if data.id is not set, which you are setting in stratumSubscribe, stratumAuthorize and stratumSubmit.

https://github.com/pocesar/node-stratum/blob/c4decc5be6c66173eac7a5ce76de576c9e9a184c/src/client.ts#L320

I did try waiting for the subscribe promise to resolve before digging into the code and discovering that the ids were not being incremented

[pocesar]

my bad, the code I wrote is wrong and the README is also wrong (for the client part). you should only call those on the mining event like so:

client.on('mining', function(req, socket, type, name) {
  // this is an answer from the server
  switch (name) {
    case 'mining.subscribe':
      socket.stratumAuthorize('user', 'pass');
      break;
    case 'mining.authorize': 
      // do more stuff
      break;
  }
})

client.connect(8080, 'localhost').then(function(socket){
  socket.stratumSubscribe('NodeMiner')
})

since I'm rewriting this library, I'll also add some docs and better examples, and make the API more intuitive

[jla415]

This works kind of works, but you are still reusing the request id as 1 in the authorize request and when the second response comes back with the same id it is not correctly recognized

from pool.on('mining.error'): Error: No suitable command was issued from the server pool.on('mining'): {"id":1,"error":null,"result":true} "result" undefined

I get what you are trying to do but the request id does need to be incremented somewhere and it is not currently doing it

[pocesar]

indeed, and there's also a bug because of this line https://github.com/pocesar/node-stratum/blob/master/src/client.ts#L145

the pending ID gets deleted, and when a new command is issued, the ID still exists (because event emitting is actually synchronous), and when the socket.stratumAuthorize is called, this.currentId is still the old one, so yes, your patch is correct, I apologize. I'll merge with my local copy, but will keep this opened for now

[jla415]

gonna close this since i'm adding other stuff to my fork and don't want to pollute your rewrite... thanks