kredits/hubot-kredits
9
0
Fork 0
Code Issues 8 Pull Requests Releases 9 Activity

Use new ethers.js NonceManager to handle transaction nonces #59

Merged
bumi merged 3 commits from ethers-nonce-manager into master 2020-07-18 10:58:45 +00:00
Conversation 11 Commits 3 Files Changed 4 +1519 -615
bumi commented 2020-06-29 16:20:55 +00:00 (Migrated from github.com)
Copy Link

So far we have failed to globally handle the transaction nonces.
The new ethers.js v5 comes with a NonceManager that helps us handling transaction nonces and automatically increases the nonce for each transaction.

We should first release: https://github.com/67P/kredits-contracts/pull/200

TODO:

  • test
  • release kredits-contracts and use released kredits-contracts version
So far we have failed to globally handle the transaction nonces. The new ethers.js v5 comes with a NonceManager that helps us handling transaction nonces and automatically increases the nonce for each transaction. We should first release: https://github.com/67P/kredits-contracts/pull/200 TODO: - [x] test - [x] release kredits-contracts and use released kredits-contracts version
raucao (Migrated from github.com) reviewed 2020-06-29 16:20:55 +00:00
raucao (Migrated from github.com) reviewed 2020-07-05 15:12:30 +00:00
integrations/zoom.js
raucao (Migrated from github.com) commented 2020-07-05 15:12:06 +00:00
Copy Link

I thought this is exactly what the nonce management would solve. We do wait for that long in all the other integrations and never had an issue with nonces as a result (only with warnings, but not with missing contribution records).

I thought this is exactly what the nonce management would solve. We do wait for that long in all the other integrations and never had an issue with nonces as a result (only with warnings, but not with missing contribution records).
raucao commented 2020-07-05 15:40:07 +00:00 (Migrated from github.com)
Copy Link

I just tried out this branch with the Gitea test repo/issues, but I'm getting a revert exception on the tx. Looks the same as when we get the chat notification, except it didn't create the contribution on my devchain (which it does on master even when it complains).

I just tried out this branch with the Gitea test repo/issues, but I'm getting a revert exception on the tx. Looks the same as when we get the chat notification, except it didn't create the contribution on my devchain (which it does on master even when it complains).
bumi commented 2020-07-06 09:28:18 +00:00 (Migrated from github.com)
Copy Link

thanks for testing. does it work for you with the master branch?
wondering what causes the revert (error in the contract)... might be because no gas limit is set - we had seen this error various times on the devchain before.

thanks for testing. does it work for you with the master branch? wondering what causes the revert (error in the contract)... might be because no gas limit is set - we had seen this error various times on the devchain before.
bumi (Migrated from github.com) reviewed 2020-07-06 09:50:03 +00:00
integrations/zoom.js
bumi (Migrated from github.com) commented 2020-07-06 09:50:02 +00:00
Copy Link

yeah, the NonceManager keeps a track of the nonces...
normally ethers.js tries to get the pending transactions count to calculate the nonce before signing it. This can fail if more transactions are created in a short time. The NonceManager should solve this by keeping track of the number of created tranactions.

I just wanted to be super save here for the next deploy that it works. thus I kept that sleep in there.

yeah, the NonceManager keeps a track of the nonces... normally ethers.js tries to get the pending transactions count to calculate the nonce before signing it. This can fail if more transactions are created in a short time. The NonceManager should solve this by keeping track of the number of created tranactions. I just wanted to be super save here for the next deploy that it works. thus I kept that sleep in there.
raucao commented 2020-07-06 15:03:21 +00:00 (Migrated from github.com)
Copy Link

Same on master when running locally, indeed.

But we really need to be able to run and develop with the devchain.

Same on master when running locally, indeed. But we really need to be able to run and develop with the devchain.
raucao commented 2020-07-06 15:09:15 +00:00 (Migrated from github.com)
Copy Link

By the way, this his how you can easily run hal7000/hubot-kredits locally yourself:

  1. In hubot-kredits, with correct branch checked out: npm link
  2. In hal8000: npm link hubot-kredits
  3. In hal8000's run.sh: add local DAO address to env var near bottom of script
  4. ./run.sh and npm run ngrok (the former gives you an interactive console/chat room, the latter catches hooks to hubot-dev.kosmos.org on your machine)
  5. Activate dev webhook in https://gitea.kosmos.org/org/kosmos/settings/hooks/4
  6. Re-open and close e.g. kosmos/test-one-two#5 (which is blacklisted in prod) in order to create contributions
By the way, this his how you can easily run hal7000/hubot-kredits locally yourself: 1. In hubot-kredits, with correct branch checked out: `npm link` 2. In hal8000: `npm link hubot-kredits` 3. In hal8000's `run.sh`: add local DAO address to env var near bottom of script 3. `./run.sh` and `npm run ngrok` (the former gives you an interactive console/chat room, the latter catches hooks to `hubot-dev.kosmos.org` on your machine) 4. Activate dev webhook in https://gitea.kosmos.org/org/kosmos/settings/hooks/4 5. Re-open and close e.g. https://gitea.kosmos.org/kosmos/test-one-two/issues/5 (which is blacklisted in prod) in order to create contributions
galfert commented 2020-07-07 14:02:13 +00:00 (Migrated from github.com)
Copy Link

By the way, this his how you can easily run hal7000/hubot-kredits locally yourself:

That's pretty helpful. Shouldn't that go into the readme?

> By the way, this his how you can easily run hal7000/hubot-kredits locally yourself: That's pretty helpful. Shouldn't that go into the readme?
👍 1
raucao commented 2020-07-08 07:12:23 +00:00 (Migrated from github.com)
Copy Link

Good point. I added a new page to the wiki: https://wiki.kosmos.org/Kredits:Development

Please liberally add more information there!

Good point. I added a new page to the wiki: https://wiki.kosmos.org/Kredits:Development Please liberally add more information there!
bumi commented 2020-07-17 12:09:25 +00:00 (Migrated from github.com)
Copy Link

zoom is a bit more annoying to test. so I'd suggest to take the risk and push this to production and handle any potential issues in new PRs.

zoom is a bit more annoying to test. so I'd suggest to take the risk and push this to production and handle any potential issues in new PRs.
👍 1
raucao commented 2020-07-17 12:29:49 +00:00 (Migrated from github.com)
Copy Link

Missing a kredits label for the PR to be ready to merge...

Missing a kredits label for the PR to be ready to merge...
Sign in to join this conversation.
Reviewers
No Reviewers
raucao
Labels
Clear labels
bug
Something is not working
dev environment
Config, builds, CI, deployment, etc.
docs
Documentation
duplicate
This issue or pull request already exists
enhancement
Improving existing functionality
feature
New functionality
idea
Something to consider
invalid
Not a bug
kredits-1
Small contribution
kredits-2
Medium contribution
kredits-3
Large contribution
question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
ui/ux
User interface, process design, etc.
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero bumi fsmanuel galfert greg haythem hueso raucao slvrbckt
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/hubot-kredits#59
No description provided.
Delete Branch "ethers-nonce-manager"

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?