Skip to content

134 remove accounttype #135

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

Merged
merged 34 commits into from
Aug 12, 2019
Merged

134 remove accounttype #135

merged 34 commits into from
Aug 12, 2019

Conversation

andy-shi88
Copy link
Contributor

Description

Breaking changes: REMOVE ACCOUNT TYPE

Breakdown

  • Remove account type usage totally
  • Update query string
  • Update transaction implementation
  • Update tests

Reference Issue

Close #134

Step to Test (optional)

  • go test ./...
  • go run main.go as usual
  • step 3

andy-shi88 and others added 28 commits August 9, 2019 12:41
…of github.com:zoobc/zoobc-core into 134-remove-accounttype
@andy-shi88
Copy link
Contributor Author

mempool not finished, sorry, back to WIP

@andy-shi88 andy-shi88 added the WIP label Aug 12, 2019
@andy-shi88 andy-shi88 removed the WIP label Aug 12, 2019
Version: 1,
TransactionType: util.ConvertBytesToUint32(txTypeMap["sendMoney"]),
Timestamp: time.Now().Unix(),
SenderAccountAddressLength: uint32(len("BCZnSfqpP5tqFQlMTYkDeBVFWnbyVK7vLr5ORFpTjgtN")),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need Address length here? because Account Address is already a string, not bytes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we do, it'll not be inserted into database, but we'll need it to parse TransactionBytes back to Transaction Object.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, but here you are passing the length of the "string" not the length of the "bytes of string"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

CREATE TABLE IF NOT EXISTS "account_balance" (
"account_id" BLOB,
"account_address" VARCHAR(255),
Copy link
Contributor

Choose a reason for hiding this comment

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

we have to keep note about this, that max address string length allowed in our system is 255

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @bzpython mentioned one blockchain that uses different encoding format (something like tribit, that the representation is of 27 characters)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's put this into note and talk about this on our next sprint meeting, in case it changes, it should be as simple as updating the migration

NodeAddress: "10.10.0.1",
LockedBalance: 10000000000,
Poown: poown,
AccountAddressLength: uint32(len("BCZnSfqpP5tqFQlMTYkDeBVFWnbyVK7vLr5ORFpTjgtN")),
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really need address length here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, reason mentioned on previous comment answer

Copy link
Contributor

Choose a reason for hiding this comment

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

check the previous reply

tx.SenderAccountType,
tx.SenderAddress,
),
"account_id": tx.SenderAddress,
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be account_address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes, fixing it.

@andy-shi88 andy-shi88 merged commit b79ebbc into develop Aug 12, 2019
@andy-shi88 andy-shi88 deleted the 134-remove-accounttype branch August 12, 2019 05:30
@andy-shi88 andy-shi88 removed their assignment Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tech: Changes - Account Type to Account Address Only
3 participants