docs: Add support for emoji reactions to posts. #391

MichelMajdalani · 2022-08-22T02:23:05Z

What is done?
As part of the documentation (walkthrough) for a first core change, I added the required code changes a new user needs to perform to add support to emoji reactions to posts.

Why is it done?
The documentation for making the first core change is missing in the docs.

How is it tested?
This is still WIP. It hasn't been tested.

Linked discussion:

Documentation PR: add serialization/deserialization react transaction docs#50

lazynina · 2022-08-23T17:32:32Z

lib/block_view_flush.go

+			// If the LikeEntry has isDeleted=true then there's nothing to do because
+			// we already deleted the entry above.
+		} else {
+			// If the LikeEntry has (isDeleted = false) then we put the corresponding


Suggested change

// If the LikeEntry has (isDeleted = false) then we put the corresponding

// If the ReactEntry has (isDeleted = false) then we put the corresponding

lazynina · 2022-08-23T17:32:45Z

lib/block_view_flush.go

+	for _, reactEntry := range bav.ReactionKeyToReactionEntry {
+
+		if reactEntry.isDeleted {
+			// If the LikeEntry has isDeleted=true then there's nothing to do because


Suggested change

// If the LikeEntry has isDeleted=true then there's nothing to do because

// If the ReactEntry has isDeleted=true then there's nothing to do because

lazynina · 2022-08-23T17:33:48Z

lib/block_view_reaction.go

+	if bav.Postgres != nil {
+		reactionExists = bav.Postgres.GetReaction(reactionKey.ReactorPubKey[:], &reactionKey.ReactedPostHash, reactionKey.ReactEmoji) != nil
+	} else {
+		reactionExists = DbGetReactorPubKeyToPostHashMapping(bav.Handle, reactionKey.ReactorPubKey[:], reactionKey.ReactedPostHash, reactionKey.ReactEmoji) != nil
+	}


We should move this to the DbAdapter

lazynina · 2022-08-23T17:37:56Z

lib/block_view_reaction.go

+	if adapter.postgresDb != nil {
+		reactions := adapter.postgresDb.GetReactionsForPost(postHash)
+		for _, reaction := range reactions {
+			bav._setReactionEntryMappings(reaction.NewReactionEntry())
+		}
+	} else {
+		handle := adapter.badgerDb
+		dbPrefix := append([]byte{}, Prefixes.PrefixPostHashToReactorPubKey...)
+		dbPrefix = append(dbPrefix, postHash[:]...)
+		keysFound, _ := EnumerateKeysForPrefix(handle, dbPrefix)
+
+		// Iterate over all the db keys & values and load them into the view.
+		expectedKeyLength := 1 + HashSizeBytes + btcec.PubKeyBytesLenCompressed
+		for _, key := range keysFound {
+			// Sanity check that this is a reasonable key.
+			if len(key) != expectedKeyLength {
+				return nil, fmt.Errorf("UtxoView.GetReactuibsForPostHash: Invalid key length found: %d", len(key))
+			}
+
+			reactorPubKey := key[1+HashSizeBytes:]
+			reactKey := MakeReactionKey(reactorPubKey, *postHash, reactionEmoji)
+			bav._getReactionEntryForReactionKey(&reactKey)
+		}
+	}


write a function on DbAdapter to get Reactions for a post hash called GetReactionsForPost first and then we can simplify the below as follows

Suggested change

if adapter.postgresDb != nil {

reactions := adapter.postgresDb.GetReactionsForPost(postHash)

for _, reaction := range reactions {

bav._setReactionEntryMappings(reaction.NewReactionEntry())

}

} else {

handle := adapter.badgerDb

dbPrefix := append([]byte{}, Prefixes.PrefixPostHashToReactorPubKey...)

dbPrefix = append(dbPrefix, postHash[:]...)

keysFound, _ := EnumerateKeysForPrefix(handle, dbPrefix)

// Iterate over all the db keys & values and load them into the view.

expectedKeyLength := 1 + HashSizeBytes + btcec.PubKeyBytesLenCompressed

for _, key := range keysFound {

// Sanity check that this is a reasonable key.

if len(key) != expectedKeyLength {

return nil, fmt.Errorf("UtxoView.GetReactuibsForPostHash: Invalid key length found: %d", len(key))

}

reactorPubKey := key[1+HashSizeBytes:]

reactKey := MakeReactionKey(reactorPubKey, *postHash, reactionEmoji)

bav._getReactionEntryForReactionKey(&reactKey)

}

}

reactions := adapter.GetReactionsForPost(postHash)

for _, reaction := range reactions {

reactKey := MakeReactionKey(reaction.ReactorPubKey, reaction.PostHash)

if _, exists := bav.ReactionKeyToReactionEntry[reactKey]; !exists {

bav._setReactionEntryMappings(reaction.NewReactionEntry())

}

}

lazynina · 2022-08-23T17:38:45Z

lib/block_view_reaction.go

+	// Iterate over the view and create the final list to return.
+	var reactorPubKeys [][]byte
+	for _, reactionEntry := range bav.ReactionKeyToReactionEntry {
+		if !reactionEntry.isDeleted && reflect.DeepEqual(reactionEntry.ReactedPostHash[:], postHash[:]) {


I think we'll want to use bytes.Equal instead of reflect.DeepEqual here since we're comparing bytes after you use the [:] operator

lazynina · 2022-08-23T19:08:22Z

lib/db_utils.go

+func (txnMeta *ReactTxindexMetadata) RawDecodeWithoutMetadata(blockHeight uint64, rr *bytes.Reader) error {
+	var err error
+
+	txnMeta.IsRemove, err = ReadBoolByte(rr)


I usually put isRemove as the last one. I think logically we would expect PostHashHex then EmojiReaction then IsRemove

lazynina · 2022-08-23T19:09:34Z

lib/network.go

+	// Set to true when a user is requesting to "remove" a reaction.
+	IsRemove bool
+
+	// The Unicode for the emoji reaction.
+	EmojiReaction rune


nit: I would flip these two

lazynina · 2022-08-23T19:10:31Z

lib/network.go

+	// Add IsRemove
+	data = append(data, BoolToByte(txnData.IsRemove))
+
+	// Add EmojiReaction.
+	// It is possible for a single character to be encoded with different code point sequences.
+	// By normalizing the Unicode (NFC), we ensure that a character will have a unique code point sequence.
+	data = append(data, norm.NFC.Bytes([]byte(string(txnData.EmojiReaction)))...)


I would flip these two. Also, will the EmojiReaction always be the same exact length of bytes? if not, we will want to prepend the number of bytes.

lazynina · 2022-08-23T19:11:46Z

lib/postgres.go

+	TransactionHash *BlockHash `pg:",pk,type:bytea"`
+	PostHash        *BlockHash `pg:",type:bytea"`
+	IsRemove        bool       `pg:",use_zero"`
+	EmojiReaction   rune       `pg:",type:integer"`


Are you sure this is an integer? I just frankly don't know off the top of my head.

lazynina · 2022-08-23T19:12:46Z

lib/postgres.go

+	}
+	return reacts
+}
+


You'll also want to write a migration for postgres in the migrations directory

This was causing nodes to reject other nodes as sync peers when they have --sync-type=blocksync but --hypersync=false even though these nodes are valid sync peers.

* Add support for a Get Single Derived Key endpoint * Make _getDerivedKeyMappingForOwner public

…deso-protocol#379) * Test metamask spending limits encoding * Add public key recovery for derived keys * test for global spending limit * Remove the test * Test refactor * Refactor signature verification * Postgres testing framework; unlimited derived keys; postgres migration; some tests * Add comments on tests; finish testing signatures * connect logic for unlimited derived keys * Finish testing * gofmt * Fix indentation * Make verify ETH signature public * Fix nilptr; improve testing functions * Review round updates; better comments and simplifications * Review round * review round 1 * Refactor * review round updates * Update go version to 1.18 * Finish review round; hard-hardcore unlimited spending limit test * Update ci.yml go version * Reset block height * Refactor signature logic to abstract away btcec's weird CompactSignature * Fix tiny error-handling bug * Add --force-checksum flag and test * Public key recovery in signature verification (deso-protocol#380) * Add public key recovery for derived keys * Review round * P/k group muting (deso-protocol#394) * added MuteList to MessagingGroupEntry struct * added TxnType validation in _connectMessagingGroup() * added MuteList to MessagingGroupEntry (for txns adding new members) * added muting and unmuting mechanism to _connectMessagingGroup() * added MuteList to RawEncodeWithoutMetadata * added MuteList to RawDecodeWithoutMetadata * moved MuteList to end of MessagingGroupEntry for backwards compatibility * added clarifying comment for MuteList * corrected typo * added MuteList to memberGroupEntry HACK * changed iii to ii * added RuleErrorMessagingMemberMuted * major muting code added (needs cleanup) * cleanup comments * fixed for loop error * deleted unused inline func * added TODO for making MuteList retrieval more efficient * fixed test typo * commented out MuteList from hacked memberGroupEntry for now * go.mod random change * fixed bug * fixed all pre-testing bugs * FIXED ALL BUGS AND ADDED TESTS * cleaned up comments * 33rd waking hour and counting... * added helpful comment * fixed unmuting bug * added unmuting tests and all successful * code cleanup * added MessagingGroupOperationMute and MessagingGroupOperationUnmute constants * replaced more constants * replaced more constants * fixed deepEqual to compare byte slices and NOT PublicKeys * fixed deepEqual to compare byte slices and NOT PublicKeys AGAIN * added gated condition to have sender and recipient in ExtraData * added comment * removed code from _disconnectMessagingGroup * added blockheight gating for messages muting * fixed existingEntry.MuteList deep copy bug * added encoder migration for DeSoV3MessagesMutingMigration * fixed HUGE testnet bug and migration bug * fixed muting code positioning * fixed deep copy bug * fixed extradata operationtype bug * fixed redundant if condition * made constant for MessagingGroupOperationType * moved contains() * throwing errors when muting already muted member or unmuting already unmuted member * made concise * removed comment * added super helpful comment * temporarily changed migration version to pass tests * FIXED MAJOR ENCODE DECODE BUG * added hacked entry optimization; fixed txn.PublicKey bug * removed comment * changed optimization comment * added prefix deprecation and replacement code * added more Deprecation code * refactored db_utils funcs and created new OptimizedMessagingGroupEntry using better prefix key structure * fixed refactoring bug; added more tests for muting while blockheight below threshold * fixed new prefix name * fixed 2 nits * cleaned up 'contains' code * added test; fixed deep equal bug * added additional unmute test * fixed deep equal nit * fixed problematic loop; added test; added RuleError * added code for groupowner not allowed to mute/unmute herself * fixed conditional dup; added extra data merging * deduplicated utxoOpsForTxn * changed comment * fixed comment grammar * added enlightening comments * added groupowner sender to ganggang in tests * [stable] Release 2.2.6 * Fix IsNodeArchival flag to include SyncTypeBlockSync This was causing nodes to reject other nodes as sync peers when they have --sync-type=blocksync but --hypersync=false even though these nodes are valid sync peers. * Simplify connect logic; start making hacked member prefix more user-friendly * Testing * More thorough testing * Temporary fix for newly-added state prefix * Another fix * fix encoding * One more pass * small rename * another pass * Fix txindex and gofmt * Rename fork height * Nina review round * Fix nil utxoview fetch Co-authored-by: Keshav Maheshwari <[email protected]> Co-authored-by: lazynina <[email protected]> Co-authored-by: diamondhands <[email protected]> * nits * Remove default key signature verification after the block height * Resolve merge conflicts with main (deso-protocol#397) * diamondhands nitpicks * Refactor V3 changes (deso-protocol#398) * Refactor message updates * Rename migration * gofmt... * filter out deleted entries * Remove sig validation for default key registration (deso-protocol#399) * Remove sig validation for default key registration * Comment out test that requires errors for bad sig for default key * use fork height to remove check on default key registration validation * Remove rogue new line * Fix isDeleted * Add comment * Add txn type check Co-authored-by: Lazy Nina <[email protected]> * Fix forkheight change bug * Re-format args * more encoder migration fixes * Ignore migration block height when value is MaxUint32 Not doing this was causing an error when setting the fork height * Simplify setting IsUnlimited on spending limit object when connecting authorize derived key txn Co-authored-by: diamondhands <[email protected]> Co-authored-by: Keshav Maheshwari <[email protected]> Co-authored-by: lazynina <[email protected]> Co-authored-by: Lazy Nina <[email protected]>

…ol#401)

…#402) * Additional persisting of state and migration checksums * make public

…ajdalani/core into docs-add-emoji-reactions

WIP: Test failing because returning invalid input for valid input

fb4e83e

lazynina requested changes Aug 23, 2022

View reviewed changes

lazynina and others added 11 commits August 27, 2022 13:36

[stable] Release 2.2.6

6979acb

Fix IsNodeArchival flag to include SyncTypeBlockSync

4ba4a3a

This was causing nodes to reject other nodes as sync peers when they have --sync-type=blocksync but --hypersync=false even though these nodes are valid sync peers.

Add support for a Get Single Derived Key endpoint (deso-protocol#393)

fda9a0f

* Add support for a Get Single Derived Key endpoint * Make _getDerivedKeyMappingForOwner public

Set fork heights for DeSoUnlimitedDerivedKeysBlockHeight (deso-protoc…

1b2170e

…ol#401)

[stable] Release 2.2.7

6ab5891

[stable] Release 3.0.0

9cd6087

Additional persisting of state and migration checksums (deso-protocol…

def66be

…#402) * Additional persisting of state and migration checksums * make public

[stable] Release 3.0.1

fe94137

WIP: add DBAdapter logic

c415754

Merge branch 'docs-add-emoji-reactions' of https://github.com/MichelM…

0c71863

…ajdalani/core into docs-add-emoji-reactions

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Add support for emoji reactions to posts. #391

docs: Add support for emoji reactions to posts. #391

MichelMajdalani commented Aug 22, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

lazynina Aug 23, 2022

	// If the LikeEntry has (isDeleted = false) then we put the corresponding
	// If the ReactEntry has (isDeleted = false) then we put the corresponding

	// If the LikeEntry has isDeleted=true then there's nothing to do because
	// If the ReactEntry has isDeleted=true then there's nothing to do because

docs: Add support for emoji reactions to posts. #391

Are you sure you want to change the base?

docs: Add support for emoji reactions to posts. #391

Conversation

MichelMajdalani commented Aug 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment