-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zone shard #175
Zone shard #175
Conversation
src/constants/shards.ts
Outdated
Prime = '0x', | ||
} | ||
|
||
function fromBytes(shard: string): Shard { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that this method is not exported but I believe the function naming could be more clear by changing it to zoneFromBytes
, deriveZoneFromBytes
or something along those lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rileystephens28 I thought about that but decided fromBytes made more sense given the context. I can change it to have the context in the name though.
src/constants/zones.ts
Outdated
Hydra3 = '0x22', | ||
} | ||
|
||
function fromBytes(zone: string): Zone { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same situation as the shard fromBytes
method, let's rename for clarity.
src/providers/abstract-provider.ts
Outdated
} | ||
| { | ||
method: 'getCode'; | ||
address: string; | ||
blockTag: BlockTag; | ||
shard: string; | ||
shard: Shard; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refers to getting a smart contracts bytecode which only applies to zones
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rileystephens28 makes sense
src/providers/abstract-provider.ts
Outdated
} | ||
| { | ||
method: 'getLogs'; | ||
filter: PerformActionFilter; | ||
shard: string; | ||
shard: Shard; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logs are created by smart contracts so this should be zone
src/providers/abstract-provider.ts
Outdated
} | ||
| { | ||
method: 'getStorage'; | ||
address: string; | ||
position: bigint; | ||
blockTag: BlockTag; | ||
shard: string; | ||
shard: Shard; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method relates to returning smart contract data at some storage position so this should be zone
*/ | ||
export interface ProtoAccessList { | ||
access_tuples: Array<ProtoAccessTuple>; | ||
} | ||
|
||
/** | ||
* @TODO write documentation for this interface. | ||
* | ||
* @category Transaction | ||
* @todo Write documentation for this interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate todo line
@@ -188,17 +189,16 @@ type allowedSignatureTypes = Signature | string; | |||
* much (the {@link ProtoTransaction.value | **value** } in ether) the operation should entail. | |||
* | |||
* @category Transaction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate category line
src/wallet/hdwallet.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file will be replaced by @alejoacosta74 updated wallet implementation feature very soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file should stay there for now though so when he rebases he can see how the enums should be used
src/wallet/qi-hdwallet.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file will be replaced by @alejoacosta74 updated wallet implementation feature very soon
src/wallet/quai-hdwallet.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file will be replaced by @alejoacosta74 updated wallet implementation feature very soon
40633fc
to
1d2b8b1
Compare
No description provided.