Use Address type #919
Use Address type #919
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
| } else { | ||
| const acc = await this._state.getAccount(message.caller) | ||
| const newNonce = acc.nonce.subn(1) | ||
| addr = generateAddress(message.caller, newNonce.toArrayLike(Buffer)) | ||
| addr = generateAddress(message.caller.buf, newNonce.toArrayLike(Buffer)) |
There was a problem hiding this comment.
Can we make generateAddress and generateAddress2 also take Address as input type?
There was a problem hiding this comment.
unfortunately those have been in ethereumjs-util for a while so we'd have to make breaking changes there. it would be nice if they output an Address as well.
another thing I noticed is it would be better if it accepted nonce as a BN instead of Buffer.
There was a problem hiding this comment.
Ah woops, thought these were defined inside EVM
| const result = await vm.runCall({ | ||
| caller: Buffer.from('0000000000000000000000000000000000000000', 'hex'), | ||
| caller: Address.zero(), |
There was a problem hiding this comment.
Few nitpick comments, but looks good in general, I do have two main points though:
- Any semi-exposed functions which use a
Bufferas input where anAddressis expected should be changed to functions which takeAddressas input (seems like only a few of those remain). This makes these changes consistent. Only use theAddress.bufif you really need it. - I am not entirely convinced yet that the VM cannot internally hit
assert(buf.length === 20, 'Invalid address length')(constructor ofAddress). If we safely convertBNs toBufferviaaddressToBufferthis should be fine.
holgerd77
force-pushed
the
use-address-type
branch
from
be15570 to
6ed7b2a
Compare
There was a problem hiding this comment.
Thanks Ryan, this looks wonderful! 🎉
Will merge this in, the suggestions from @jochem-brouwer can be done after the dev release since not life-threatening, I actually expect at least 2-3 rounds of dev releases with all the changes we've done.
This PR updates the vm to use the
Addresstype internally and externally to improve clarity and provide strict byte length validation.