Skip to content
This repository has been archived by the owner on Mar 3, 2021. It is now read-only.

Fixed a bug with the wrong event name #1347

Merged
merged 2 commits into from
Dec 4, 2019
Merged

Conversation

EthanWan
Copy link

@EthanWan EthanWan commented Dec 3, 2019

The debug at Remix-ide of the new version can't display full storage changes, emmm... i think this is the reason

@Aniket-Engg Aniket-Engg self-requested a review December 3, 2019 10:57
@Aniket-Engg
Copy link
Collaborator

Hello @EthanWan , Thanks for the PR. Can you please add more details and reference to your changes?

@EthanWan
Copy link
Author

EthanWan commented Dec 4, 2019

Hello @EthanWan , Thanks for the PR. Can you please add more details and reference to your changes?

Recently, when i use remix-ide of new version to debug my contract. i found that the full storages changes not display (it's good in old version). as shown in below img
691575427228_ pic_hd

So. I went to check the remix-ide source code. the code to get full storages is:

this.vmDebuggerLogic.event.register('traceStorageUpdate', this.fullStoragesChangesPanel.update.bind(this.fullStoragesChangesPanel))

But when i check remix-debug's code. I can't find trigget for this event which name is traceStorageUpdate. by reading the code i think it's the wrong event name. So I changed it in my commit

@EthanWan
Copy link
Author

EthanWan commented Dec 4, 2019

After testing, my commit was not enough to fix the problem and i still need to change some code to make it works.
The complete code is as follows (remix-debug/src/debugger/VmDebugger.js)

self.event.register('indexChanged', this, function (index) {
  if (index < 0) return
  if (self.stepManager.currentStepIndex !== index) return
  if (!self.storageResolver) return

  if (index !== self.traceLength - 1) {
    return self.event.trigger('traceStorageUpdate', [{}])
  }
  var storageJSON = {}
  for (var k in self.addresses) {
    var address = self.addresses[k]
    var storageViewer = new StorageViewer({ stepIndex: self.stepManager.currentStepIndex, tx: self.tx, address: address }, self.storageResolver, self._traceManager)
    storageViewer.storageRange(function (error, result) {
      if (!error) {
        storageJSON[address] = result
        self.event.trigger('traceStorageUpdate', [storageJSON])
      }
    })
   }
  })

By the way, I have a question, why need add this judgment

if (index !== self.traceLength - 1) {
  return self.event.trigger('traceStorageUpdate', [{}])
}

@Aniket-Engg
Copy link
Collaborator

Aniket-Engg commented Dec 4, 2019

Thanks for the great explanation @EthanWan . I would like to ask that after these changes, were you able to see the data in "Full Storage Changes" section of IDE. I tried with your changes but there seems no changes.

@EthanWan
Copy link
Author

EthanWan commented Dec 4, 2019

Thanks for the great explanation @EthanWan . I would like to ask that after these changes, were you able to see the data in "Full Storage Changes" section of IDE. I tried with your changes but there seems no changes.

Yes i can. but just the last step of debugging
WechatIMG70

Because of this judgment, but I don’t know why it was added

if (index !== self.traceLength - 1) {
  return self.event.trigger('traceStorageUpdate', [{}])
}

@Aniket-Engg
Copy link
Collaborator

That check should be handled, will do in my next PR. Thanks for your awesome work. 🎉

Copy link
Collaborator

@Aniket-Engg Aniket-Engg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@Aniket-Engg Aniket-Engg merged commit d35ac4e into ethereum:master Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants